feature request: add `--destdir` option to install.sh about arttime HOT 6 CLOSED

So it makes sense to delay this till its really needed.

Sure.

The exit 0 is required because the script exits with a non zero signal causing the build framework to immediately fail. Changing TERM in the build environment is no option.

from arttime.

Confirmed. 6716e6b won't require me patching in an exit 0 in the future. Thanks!

from arttime.

@ehaupt I added a switch --noupdaterc for the second problem you mention. If passed, installer won't add anything to rc files. 0f79226. That way you don't need to patch anything.

For the first problem, I am not sure what exactly is needed here? Why can't the --prefix you pass to the installer be /usr/ports/deskutils/arttime/work/stage/usr/local?

Like this (assuming DESTDIR=/usr/ports/deskutils/arttime/work/stage):

./install.sh --noupdaterc --prefix $DESTDIR/usr/local --zompdir $DESTDIR/usr/local/share/zsh/functions
# OR
./install.sh --noupdaterc --prefix $DESTDIR/usr/local --zompdir $DESTDIR/usr/share/zsh/functions

From your description it looks like you are just adding a constant value to an existing prefix option. If I am missing something please let me know...

from arttime.

In the case of arttime's installer the --prefix argument might suffice unless it's value is used to adjust paths within the projects code.

Consider this:

I use --prefix=/mnt/sandbox/usr/local/share/zsh/functions and during the installation/building prefix is also used to adjust paths within the code.

Now you enter the chroot (in my example /mnt/sandbox). You execute the script and it won't find the paths that have been modified to /mnt/sandbox/usr/local/share/zsh/functions because within the chroot the files are located under /usr/local/share/zsh/functions

Again, this is not the case here. But its generally a good practice to have something like destdir for sandbox use in case path substitution is done at some point.

I hope that makes sense. Maybe this explains it better.

from arttime.

@ehaupt Please pardon my lack of understanding of installers, though what do you mean by adjust paths within project code?

In the case of arttime's installer the --prefix argument might suffice unless it's value is used to adjust paths within the projects code.

From your comments it looks like it is not necessary to have DESTDIR prefix for arttime's installer? If I got it right then let's refrain from adding it at the moment. In future if I am doing a "adjust paths" (which I know nothing about), then it would make most sense to add DESTDIR. My concern is that if I add it now without fully understanding what can go wrong, I might break something in future and request you to review anyway. So it makes sense to delay this till its really needed.

One last thing I notice in your installer patch is the addition of exit 0 at the end. Did you have to do that because there was some kind of error in a CI run for installer? I noticed that Homebrew's CI would fail because echoti cnorm (last line in install.sh) would return error as TERM was not really set there. In a reasonable machine, TERM would be set to vt100 or its derivative. Check my comment there: Homebrew/homebrew-core#124403 (comment). If you added exit 0 to bypass problems with echoti, then I would recommend setting TERM to xterm. In future I will anyway refactor the code so this would work fine on CI environments that don't set TERM to a reasonable value.

from arttime.

@ehaupt With this change: 6716e6b, installer would not exit with error due to missing term capability. So hopefully you would not require patching with exit 0 anymore... Let me know if you need anything else to avoid patching install.sh. I think one more thing would be to avoid printing some messages as they won't make sense, especially ones that ask user to add some directory to PATH or fpath.

from arttime.