Description of problem: There is a major regression tcsh logic with this tcsh release. A single-line if statement like: if (1) source /tmp/somefile.csh fails to source the file. This is critical, since it's *very* common to have clauses like: if ( -e ~/.alias ) source ~/.alias in csh scripts. This has a pretty high breakage footprint throughout our enterprise. Version-Release number of selected component (if applicable): tcsh-6.17-19.el6_2.src.rpm How reproducible: Always. In fact, here's a trivial reproducer script "testit.csh" Actual results: ----- % cat testit.csh echo 'echo "source worked"' > /tmp/foo.csh echo "test start" if (1) source /tmp/foo.csh echo "test done" % /bin/tcsh -f testit.csh test start test done % ----- Expected results: ----- % cat testit.csh echo 'echo "source worked"' > /tmp/foo.csh echo "test start" if (1) source /tmp/foo.csh echo "test done" % /bin/tcsh -f testit.csh test start source worked test done % ----- As you can see, sourcing /tmp/foo.csh should have resulted in a "source worked" output, which it doesn't in the problem case. Additional info: We grabbed the source rpm and tracked it down to a problem with this patch: Patch31: tcsh-6.17.00-history-file-locking.patch Commenting out that patch corrects the issue. That patch is associated with bug 648592, which unfortunately is not public. This is pretty distressing, since this is the second time in row that a Red Hat update has gone out with a non-functioning tcsh due to a Red Hat introduced bug. The effect of RHEL 6.2 Bug 658190 "fix" was corrected with this release, but tcsh still isn't usable due to the introduction of this new logic error.
Here's a quick patch to the spec file that removes the problem patch. When a fixed version is released via RHN, please bump the release number past 20 so we'll be sure to pick up the updated package. We've got to push this internal release to stay in production. --- rpmbuild/SPECS/tcsh.spec_orig 2012-05-10 06:19:18.000000000 -0700 +++ rpmbuild/SPECS/tcsh.spec_revert 2012-08-09 16:41:21.000000000 -0700 @@ -3,7 +3,7 @@ Summary: An enhanced version of csh, the C shell Name: tcsh Version: 6.17 -Release: 19%{?dist} +Release: 20%{?dist} License: BSD Group: System Environment/Shells Source: ftp://ftp.astron.com/pub/tcsh/%{name}-%{version}.00.tar.gz @@ -28,7 +28,7 @@ Patch25: tcsh-6.17.00-child-kill-hang.patch Patch26: tcsh-6.17.00-variable-names.patch Patch30: tcsh-6.17.00-handle-signals-before-flush.patch -Patch31: tcsh-6.17.00-history-file-locking.patch +#Patch31: tcsh-6.17.00-history-file-locking.patch Patch32: tcsh-6.17.00-posix-exit-status-value.patch Provides: csh = %{version} @@ -69,7 +69,7 @@ %patch25 -p1 -b .child-kill-hang %patch26 -p1 -b .variable-names %patch30 -p1 -b .handle-signals-before-flush -%patch31 -p1 -b .history-file-locking +#%patch31 -p1 -b .history-file-locking %patch32 -p1 -b .posix-exit-status-value for i in Fixes WishList; do @@ -144,6 +144,10 @@ %{_mandir}/man1/*.1* %changelog +* Thu Aug 09 2012 Bob Arendt <rda> - 6.17-20 +- Remove history-file-locking.patch + Breaks single line "if (..) source <somefile>" tcsh logic (#847102) + * Thu May 10 2012 Vojtech Vitek (V-Teq) <vvitek> - 6.17-19 - Revert srcfile behaviour to return backward-compatible error codes (#658074) ---------------------------------
*** Bug 826140 has been marked as a duplicate of this bug. ***
Created attachment 606312 [details] Proposed patch to fix the source command The problem is caused by calling dosource() through a function pointer which doesn't have the flg argument. Other builtins don't need it, so I added an exception for dosource so it's now called directly.
Created attachment 606398 [details] Alternate patch for dosource() (In reply to comment #8) > Created attachment 606312 [details] > Proposed patch to fix the source command Good find - that K&R style code looks like it has declaration prototypes disabled. It makes it easy to supply the wrong argument count. The only issue with this and the previous history patch is that they break builds on some other platforms which *do* use prototypes: From sh.h: 942 #if defined(hpux) && defined(__STDC__) && !defined(__GNUC__) 943 /* Avoid hpux ansi mode spurious warnings */ 944 typedef void (*bfunc_t) (); 945 #else 946 typedef void (*bfunc_t) (Char **, struct command *); 947 #endif /* hpux && __STDC__ && !__GNUC__ */ Some non-__GNUC__ plaforms will get prototypes. So all the commands should only have 2 args. Although these platforms may be unsupported by RedHat, it's unlikely that upstream would accept patches that broke other architectures. Also it might be nice if prototypes were enabled for gcc; It would certainly make maintanence easier. Looking through the source, it looks like the thing to do is rename dosource to dosource_lck(), using the 3-arg dosource_lck() for all the direct calls in the code. Then make a dosource() wrapper for dosource_lck() with flg=0. So all source file reads are not locked - historically true, and should be pretty safe. I've attached a patch that implements this. I've built a rpm and hope to get some testing in on it. Unfortunately we don't have a rigorous test suite .. we just seem to get bitten by bugs.
*** Bug 828772 has been marked as a duplicate of this bug. ***
*** Bug 857019 has been marked as a duplicate of this bug. ***
Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory, and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. http://rhn.redhat.com/errata/RHBA-2013-0446.html