Add sdh_install helper function to sage-dist-helpers

[embray]

Adds a new `sdh_install` helper function:

This is like a very simplified version of the `install` program, and is
intended for use by packages whose spkg-install just copies some files (or
more complex packages that copy some files as one of its steps).  The
majority of its functionality is just in ensuring that the full destination
path exists, and to provide some error checking.  This replaces some
patterns that occur frequently in some spkg-installs.

Also updates the mathjax and thebe packages as two examples of its usage (this implements #24024 for those packages).

Component: build

Keywords: destdir mathjax thebe

Author: Erik Bray

Branch/Commit: 0f848c3

Reviewer: Jeroen Demeyer

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

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

330e618	Introduce SAGE_DESTDIR_LOCAL variable
9d14972	Don't install gcc as part of gfortran
5f8549e	Merge branch 'u/jdemeyer/gfortran_breaks_parallel_build' into u/embray/build/sdh_install_files
22c3e9e	fix bug in handling of the -T flag--it was as if it was always set

[sagetrac-git]

Changed commit from 6a4e33a to 22c3e9e

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

6c8bd46	fix bug in handling of the -T flag--it was as if it was always set
6dcb880	fix bug that can occur in repeated invocations of sdh_install in the same script

[sagetrac-git]

Changed commit from 22c3e9e to 6dcb880

[jdemeyer]

comment:4

1. I don't really understand the point of

    if [ $# -eq 0 ]; then
        sdh_die "At least a source and destination file are required"
    fi

since it would fail anyway later and no error message would be given for sdh_install -T.

2. It looks strange to put this in an else block, it's just part of the normal flow of execution:

    else
        dest="${SAGE_DESTDIR}$dest"
    fi

3. I seem to recall that cp -R is more portable than cp -r. So unless you have a good reason for using cp -r, I recommend cp -R.

4. For consistency, I would start all sdh_die messages with Error:

5. echo "Installing files for ${PKG_NAME}:" looks like unneeded verbosity. I would drop that.

6. If mkdir or cp fails, it already prints an error message, so I don't need that we need sdh_die for that. I would just write cp ... || exit $?.

[embray]

comment:5

Replying to @jdemeyer:

1. I don't really understand the point of

    if [ $# -eq 0 ]; then
        sdh_die "At least a source and destination file are required"
    fi

since it would fail anyway later and no error message would be given for sdh_install -T.

I mean, it will still fail for sdh_install -T because the $src variable will be empty. I thought an explicit check of "there are arguments" would enhance clarity but in retrospect I agree it's superfluous.

2. It looks strange to put this in an else block, it's just part of the normal flow of execution:

    else
        dest="${SAGE_DESTDIR}$dest"
    fi

Yep. I think that was just the result of remolding of slightly different logic.

3. I seem to recall that cp -R is more portable than cp -r. So unless you have a good reason for using cp -r, I recommend cp -R.

I suspect it's not really a problem for us since I've already seen cp -r in some of the spkg-installs I've used this in--including standard packages. If that were a problem for a platform we care about it would have come up by now. However, I seem to recall the same now that you mention it, so I could change it.

4. For consistency, I would start all sdh_die messages with Error:

+1

5. echo "Installing files for ${PKG_NAME}:" looks like unneeded verbosity. I would drop that.

+1 Some of the other sdh_ commands had some kind of message like this so I put it in for consistency. But after trying this on a few packages it hasn't been too helpful (especially in cases where there are multiple sdh_install calls in the package).

6. If mkdir or cp fails, it already prints an error message, so I don't need that we need sdh_die for that. I would just write cp ... || exit $?.

A lot of the code this function is replacing was also adding its own error messages on top of mkdir and/or cp, so this was just carried over from that. In most cases those programs' error messages should be sufficient though.

[sagetrac-git]

Changed commit from 6dcb880 to 42726a2

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

42726a2

various minor review comments

[jdemeyer]

comment:8

Wait... did you actually agree with all my comments? :-)

[jdemeyer]

Reviewer: Jeroen Demeyer

[embray]

comment:9

Replying to @jdemeyer:

Wait... did you actually agree with all my comments? :-)

Fair enough--but it's not like I set out to disagree with you. I thought they were all reasonable enough :)

[jdemeyer]

Changed branch from u/embray/build/sdh_install_files to u/jdemeyer/build/sdh_install_files

[jdemeyer]

New commits:

fa991ca

sdh_install_files -> sdh_install in error messages

[jdemeyer]

Changed commit from 42726a2 to fa991ca

[embray]

comment:12

Thanks for catching that. It was originally called sdh_install_files but I decided to shorten it later.

[embray]

Changed branch from u/jdemeyer/build/sdh_install_files to u/embray/build/sdh_install_files

[embray]

comment:13

Just rebased on current develop.

---

New commits:

27a44b4	Adds a new sdh_install helper function:
0bbff00	fix bug in handling of the -T flag--it was as if it was always set
7d09c66	fix bug that can occur in repeated invocations of sdh_install in the same script
598112b	various minor review comments
dbd9ca6	sdh_install_files -> sdh_install in error messages

[embray]

Changed commit from fa991ca to dbd9ca6

[vbraun]

comment:14

Merge conflict

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

b0b38c2	Adds a new sdh_install helper function:
852cb5f	fix bug in handling of the -T flag--it was as if it was always set
5f7cd30	fix bug that can occur in repeated invocations of sdh_install in the same script
daa37d5	various minor review comments
0f848c3	sdh_install_files -> sdh_install in error messages

[sagetrac-git]

Changed commit from dbd9ca6 to 0f848c3

[vbraun]

Changed branch from u/embray/build/sdh_install_files to 0f848c3