Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Investigate ditching Stow's pre-processing phase of installation #84

Open
aspiers opened this issue Apr 5, 2021 · 1 comment
Open
Labels
infrastructure UX Issues relating to user experience

Comments

@aspiers
Copy link
Owner

aspiers commented Apr 5, 2021

Currently Stow is built by pre-processing *.in files into corresponding files with the .in prefix dropped. For example:

  • bin/stow is generated from bin/stow.in,
  • lib/Stow.pm is generated from lib/Stow.pm.in

and so on.

Why is this done? The answer can be seen from the very simple pre-processor in Makefile.am:

edit = sed -e 's|[@]PERL[@]|$(PERL)|g'       \
           -e 's|[@]VERSION[@]|$(VERSION)|g' \
           -e "s|[@]USE_LIB_PMDIR[@]|$$use_lib_pmdir|g"

In other words, it's done in order to ensure that:

  1. the correct shebang line at the start of bin/stow and bin/chkstow is correct (#!/usr/bin/perl in many cases, but could be something else if your Perl installation is elsewhere)
  2. $Stow::VERSION and $Stow::Util::VERSION are set to the correct version number (the same as the Stow release version)
  3. bin/stow has a use lib line pointing to the location of the Stow and Stow::Util modules, so that it always finds them.

This seems to work OK, but has several disadvantages:

  • Development has to occur in *.in files, which confuses code editors.
  • Testing of code modifications requires the pre-processor to be run in between every modification and the next test run. make watch is a hack to make this slightly more convenient, but it would be better if it wasn't necessary.
  • Testing of code modifications to the libraries in lib/ not only requires the pre-processor run, but also make install, because the use lib line in the stow script ensures that the final location of the modules is first on the @INC load path at run-time. So even if you try to test by running make (not make install) and then something like perl -Ilib bin/stow, it will pick up the installed Stow.pm, not the one you just built in lib/.
  • It's a non-standard approach.
  • It just all feels more complicated than it should need to be.

One obvious question is: why on earth did I do it this way? Unfortunately I can't remember; maybe there was a good reason, or maybe I just wasn't experienced enough at the time to make the right choice. Perhaps digging through git history will answer this.

A related but more important question is: can we ditch this, and adopt a simpler, more standard approach, e.g.:

  • Remove the embedded use lib line and trust the user to set the right PERL5LIB if installing into a per-user or non-standard location which isn't system-wide.
  • Switch shebang lines to #!/usr/bin/env perl and trust the user to set the right $PATH.
  • Change Makefile.am to write the version string to a separate file such as Stow/version.pm, and import that where needed.

None of this is specific to Stow, and applies to pretty much any CPAN module, so we should probably adopt whatever is the standard approach these days.

However this may require removing support for the autotools-based installation route, and I don't know how many people use that vs. the Module::Build installation route. Worse, I don't even know how I can effectively poll the userbase to find out one way or the other. Suggestions welcome.

(It's interesting to note that other "scripting" languages such as Python and Ruby can solve this by auto-generating wrapper stub scripts.)

@aspiers aspiers added UX Issues relating to user experience infrastructure labels Apr 5, 2021
@aspiers
Copy link
Owner Author

aspiers commented Apr 7, 2024

It turns out that this pre-processing prevents being able to view test coverage on Coveralls, e.g. https://coveralls.io/builds/66764009/source?filename=bin%2Fchkstow

aspiers added a commit that referenced this issue Apr 7, 2024
Unfortunately for now, Coveralls reports don't include source due
to #84, but this is a good workaround.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure UX Issues relating to user experience
Projects
None yet
Development

No branches or pull requests

1 participant