Opened 11 years ago
Closed 11 years ago
#11663 closed defect (fixed)
singular files not world-readable
Reported by: | jdemeyer | Owned by: | tbd |
---|---|---|---|
Priority: | blocker | Milestone: | sage-4.7.1 |
Component: | packages: standard | Keywords: | |
Cc: | Merged in: | sage-4.7.1.rc2 | |
Authors: | Jeroen Demeyer | Reviewers: | Leif Leonhardy |
Report Upstream: | Reported upstream. Little or no feedback. | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
Similar to #11660, the singular files are not world-readable.
New spkg: http://boxen.math.washington.edu/home/jdemeyer/spkg/singular-3-1-1-4.p11.spkg
Reported upstream: http://www.singular.uni-kl.de:8002/trac/ticket/354
Change History (28)
comment:1 Changed 11 years ago by
comment:2 Changed 11 years ago by
- Description modified (diff)
- Status changed from new to needs_review
comment:3 Changed 11 years ago by
- Report Upstream changed from N/A to Not yet reported upstream; Will do shortly.
comment:4 Changed 11 years ago by
- Description modified (diff)
- Report Upstream changed from Not yet reported upstream; Will do shortly. to Reported upstream. Little or no feedback.
comment:5 follow-up: ↓ 7 Changed 11 years ago by
Which (of the installed) files were affected?
I only noticed
local/share/singular/singular.hlp local/share/singular/singular.idx
(and local/share/singular/help.cnf
, which currently unintentionally doesn't get installed, cf. #11519 and #5994).
Also, all (header) files in local/include/singular
are executable. :)
comment:6 Changed 11 years ago by
Note that (almost*) all files are installed with some install
script or program (not consistently making use of -m ...
though as far as I can see), so changing file permissions in src/
isn't really appropriate or may not be sufficient.
*There are actually a few (at least one) instances where cp
is used.
comment:7 in reply to: ↑ 5 ; follow-up: ↓ 14 Changed 11 years ago by
Replying to leif:
I only noticed
local/share/singular/singular.hlp local/share/singular/singular.idx
(and
local/share/singular/help.cnf
, which currently unintentionally doesn't get installed, cf. #11519 and #5994).
These two files are actually copied by spkg-install
:
install_docs() { cp $SHARED/singular.hlp $SAGE_LOCAL/share/singular/ if [ $? -ne 0 ]; then echo "Error installing documentation while copying singular.hlp" exit 1 fi cp $SHARED/singular.idx $SAGE_LOCAL/share/singular/ if [ $? -ne 0 ]; then echo "Error installing documentation while copying singular.idx" exit 1 fi }
So it's not really an upstream issue.
comment:8 Changed 11 years ago by
P.S.: The created Singular scripts in local/bin/
use $*
instead of "$@"
.
comment:9 follow-up: ↓ 10 Changed 11 years ago by
- Reviewers set to Leif Leonhardy
- Status changed from needs_review to needs_work
- Work issues set to Use `cp -p` in `spkg-install`
This doesn't work (i.e., doesn't solve the issue), since cp
is required (by POSIX) to respect the umask
unless -p
is used (which we don't, see above).
Also, janet.{cc,h}
are still executable in the upstream sources, and at least for me all headers still get installed with -rwxr-xr-x
.
While you're at it (spkg-install
), I think you should also add copying the help.cnf
file (which is as noted missing in our "manual" installations).
comment:10 in reply to: ↑ 9 Changed 11 years ago by
Replying to leif:
While you're at it (
spkg-install
), I think you should also add copying thehelp.cnf
file (which is as noted missing in our "manual" installations).
Please also quote some environment variables (at least there, in install_docs()
).
comment:11 Changed 11 years ago by
Instead of using cp -p
, you could of course also chmod
them afterwards, or simply change the umask
before copying.
In principle, we could (or should?) set the umask
in sage-spkg
.
comment:12 Changed 11 years ago by
Actually, the problem with singular.{hlp,idx}
is not my umask
, but that the files already existed (and don't get deleted prior to reinstallation), so their permissions didn't get changed.
Anyway, using cp -p
would solve this.
comment:13 Changed 11 years ago by
- Description modified (diff)
comment:14 in reply to: ↑ 7 Changed 11 years ago by
- Status changed from needs_work to needs_review
- Work issues Use `cp -p` in `spkg-install` deleted
Replying to leif:
These two files are actually copied by
spkg-install
:
It is an upstream issue because the files have permission 0600 in the source tree, so they are copied to permission 0600 (both with and without -p).
The installation of the headers as executable files is an upstream problem. I have not really pinned it down, but the Singular install scripts are full of chmod
s.
Anyway, I have a new spkg up at http://boxen.math.washington.edu/home/jdemeyer/spkg/singular-3-1-1-4.p11.spkg using cp -p
for the help files and removing an existing chmod
hack. I also made some file non-executable.
I do not want to make further changes here which are unrelated to the ticket. I agree with some of the points you make, but not for this ticket.
comment:15 follow-up: ↓ 16 Changed 11 years ago by
Hi Jeroen and Leif,
as far as I know, only a small change would be needed in the Singular spkg in order to fix #11645. Could that perhaps be included into your singular-3-1-1-4.p11.spkg as well?
comment:16 in reply to: ↑ 15 ; follow-up: ↓ 17 Changed 11 years ago by
Replying to SimonKing:
as far as I know, only a small change would be needed in the Singular spkg in order to fix #11645. Could that perhaps be included into your singular-3-1-1-4.p11.spkg as well?
I would yes but only if it does not further delay the 4.7.1 release. I see no patch for #11645, do you have one?
comment:17 in reply to: ↑ 16 ; follow-ups: ↓ 18 ↓ 19 Changed 11 years ago by
Replying to jdemeyer:
Replying to SimonKing:
as far as I know, only a small change would be needed in the Singular spkg in order to fix #11645. Could that perhaps be included into your singular-3-1-1-4.p11.spkg as well?
I would yes but only if it does not further delay the 4.7.1 release. I see no patch for #11645, do you have one?
I've posted the patch upstream ("plain" context diff):
comment:18 in reply to: ↑ 17 Changed 11 years ago by
Replying to leif:
I've posted the patch upstream ("plain" context diff):
It's now also over there: ticket:11645:trac_11645-install_gftables_one_by_one.patch
comment:19 in reply to: ↑ 17 ; follow-up: ↓ 20 Changed 11 years ago by
Replying to leif:
Replying to jdemeyer:
Replying to SimonKing:
as far as I know, only a small change would be needed in the Singular spkg in order to fix #11645. Could that perhaps be included into your singular-3-1-1-4.p11.spkg as well?
I would yes but only if it does not further delay the 4.7.1 release. I see no patch for #11645, do you have one?
I've posted the patch upstream ("plain" context diff)...
However, I can also quickly provide a new Singular spkg based on Karl-Dieter's (p12 then), based on yours, meanwhile p11. :)
Note also http://trac.sagemath.org/sage_trac/ticket/11645#comment:16; we already patch src/Singular/Makefile.in
.
comment:20 in reply to: ↑ 19 Changed 11 years ago by
- Status changed from needs_review to positive_review
Replying to leif:
Replying to leif:
I've posted the patch upstream ("plain" context diff)...
Note also http://trac.sagemath.org/sage_trac/ticket/11645#comment:16; we already patch
src/Singular/Makefile.in
.
FWIW, I've attached replacement files for patches/Singular-Makefile.in
and patches/Singular-Makefile.in.patch
to #11645, i.e. a cumulative patch to src/Singular/Makefile.in
and the corresponding pre-patched file (which currently gets copied over into the upstream source tree).
Otherwise positive review for the current p11, as it now fixes the permissions issue (i.e., files not readable by "others"), though I think we should fix this in a more general way. (We'll have to check and probably change the upstream sources again on any upstream upgrade.)
Revert the status to "needs review" in case you want to fix #11645 here, too (by simply replacing the two files in patches/
as mentioned above, and of course updating SPKG.txt
.)
comment:21 follow-up: ↓ 22 Changed 11 years ago by
- Status changed from positive_review to needs_info
I hate to spoil this party, but where was the p10? Even SPKG.txt
doesn't mention one.
Not sure if this means 'needs work', but at least 'needs info'!
comment:22 in reply to: ↑ 21 Changed 11 years ago by
Replying to kcrisman:
I hate to spoil this party, but where was the p10?
Sorry, I deleted it (before realizing that somebody might still be interested in it).
comment:23 Changed 11 years ago by
- Status changed from needs_info to needs_review
comment:24 follow-up: ↓ 26 Changed 11 years ago by
- Status changed from needs_review to positive_review
I believe kcrisman's comment should not affect the positive review...
comment:25 Changed 11 years ago by
I have posted a p12 at ticket #11645.
comment:26 in reply to: ↑ 24 Changed 11 years ago by
Replying to jdemeyer:
I believe kcrisman's comment should not affect the positive review...
Okay, then I'll try to make a p13 for #11550. Please someone just give #11645 a positive review and not 'needs work', it is much more tedious for me to update spkgs than for the more technically astute, I'm afraid :(
comment:27 Changed 11 years ago by
comment:28 Changed 11 years ago by
- Merged in set to sage-4.7.1.rc2
- Resolution set to fixed
- Status changed from positive_review to closed
Note that there's already a p10 (#11550), and trivial outstanding changes to that (#11645).