singular files not world-readable

[jdemeyer]

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

Upstream: Reported upstream. Little or no feedback.

Component: packages: standard

Author: Jeroen Demeyer

Reviewer: Leif Leonhardy

Merged: sage-4.7.1.rc2

Issue created by migration from https://trac.sagemath.org/ticket/11663

[nexttime]

comment:1

Note that there's already a p10 (#11550), and trivial outstanding changes to that (#11645).

[jdemeyer]

Author: Jeroen Demeyer

[jdemeyer]

Upstream: Not yet reported upstream; Will do shortly.

[jdemeyer]

Changed upstream from Not yet reported upstream; Will do shortly. to Reported upstream. Little or no feedback.

[nexttime]

comment:5

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. :)

[nexttime]

comment:6

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.

[nexttime]

comment:7

Replying to @nexttime:

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.

[nexttime]

comment:8

P.S.: The created Singular scripts in local/bin/ use $* instead of "$@".

[nexttime]

Reviewer: Leif Leonhardy

[nexttime]

Work Issues: Use cp -p in spkg-install

[nexttime]

comment:9

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).

[nexttime]

comment:10

Replying to @nexttime:

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).

Please also quote some environment variables (at least there, in install_docs()).

[nexttime]

comment:11

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.

[nexttime]

comment:12

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.

[jdemeyer]

Changed work issues from Use cp -p in spkg-install to none

[jdemeyer]

comment:14

Replying to @nexttime:

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 chmods.

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.

[simon-king-jena]

comment:15

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?

[jdemeyer]

comment:16

Replying to @simon-king-jena:

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?

[nexttime]

comment:17

Replying to @jdemeyer:

Replying to @simon-king-jena:

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):

http://www.singular.uni-kl.de:8002/trac/raw-attachment/ticket/352/singular-trac_352-install_gftables_one_by_one.patch

[nexttime]

comment:18

Replying to @nexttime:

I've posted the patch upstream ("plain" context diff):

http://www.singular.uni-kl.de:8002/trac/raw-attachment/ticket/352/singular-trac_352-install_gftables_one_by_one.patch

It's now also over there: attachment: ticket:11645:trac_11645-install_gftables_one_by_one.patch

[nexttime]

comment:19

Replying to @nexttime:

Replying to @jdemeyer:

Replying to @simon-king-jena:

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 #11645 comment:16; we already patch src/Singular/Makefile.in.

[nexttime]

comment:20

Replying to @nexttime:

Replying to @nexttime:

I've posted the patch upstream ("plain" context diff)...

Note also #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.)

[kcrisman]

comment:21

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'!

[jdemeyer]

comment:22

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).

[jdemeyer]

comment:24

I believe kcrisman's comment should not affect the positive review...

[jdemeyer]

comment:25

I have posted a p12 at ticket #11645.

[kcrisman]

comment:26

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 :(

[kcrisman]

comment:27

Followup p13 (based on #11645) at #11550.

[jdemeyer]

Merged: sage-4.7.1.rc2