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

Honor autoconf --prefix arguments #63

Closed
wants to merge 9 commits into from

Conversation

naterini
Copy link

@naterini naterini commented Jun 8, 2018

For our machines, we generally prefer to put compiled applications in /usr/local. This patch follows the suggestions of GNU (https://www.gnu.org/software/autoconf/manual/autoconf-2.63/html_node/Installation-Directory-Variables.html and https://www.gnu.org/software/automake/manual/html_node/Scripts.html#Scripts) to push the configure settings into the nhc scripts. The main nhc scripts had to be converted to input templates per autoconf convention.

This patch fixes issue in #62

@mej
Copy link
Owner

mej commented Jul 17, 2018

So first, any particular reason you generate the scripts from the .in files in the Makefile instead of adding them to AC_OUTPUT() in configure.ac (which is the standard method)?

I'm really torn on this one. I've kinda been avoiding doing something like this because I have a bit of a visceral reaction to turning all the base scripts into .in files, but I also understand the need and motivation to make this work.

One alternative possibility might be to transform the installed versions of the scripts rather than the sources, possibly with an automake install-exec-hook rule or something similar. Would that adequately address your use case?

@mej mej self-assigned this Jul 17, 2018
@mej mej added this to the 1.4.3 Release milestone Jul 17, 2018
@naterini
Copy link
Author

naterini commented Jul 18, 2018

So first, any particular reason you generate the scripts from the .in files in the Makefile instead of adding them to AC_OUTPUT() in configure.ac (which is the standard method)?

I was trying to follow the existing pattern of hardcoding the paths in the final installed script. I think using an environmental variable for these would be much cleaner and could easily swap this pull request to that instead. The downside to using environment could cause behavior to be surprising on a default install though but updating documentation should help with that.

I'm really torn on this one. I've kinda been avoiding doing something like this because I have a bit of a visceral reaction to turning all the base scripts into .in files

I was following the best practice suggestions from autoconf (https://www.gnu.org/software/automake/manual/html_node/Scripts.html#Scripts). I don't actually like this setup either but I assumed you would prefer follow autoconf dev's suggestions.

One alternative possibility might be to transform the installed versions of the scripts rather than the sources, possibly with an automake install-exec-hook rule or something similar.

Sure. AFAIK appending the ".in" serves as a good warning to any future developers that this file will be modified during install process.

@eliv
Copy link

eliv commented Aug 16, 2018

Yes, us to. Just Installed nhc 1.4.2 gives weird error messages until I found PATH in the nhc script. Adding /usr/local/bin to find sinfo and scontrol and I can at least start kicking the tires.

@mej mej modified the milestones: 1.4.3 Release, 1.4.4 Release Oct 30, 2018
@mej mej added bug and removed enhancement labels Oct 30, 2018
@mej mej changed the base branch from master to bug/62-autoconf-paths October 30, 2018 19:39
@mej
Copy link
Owner

mej commented Oct 30, 2018

I created a new branch for applying this PR so that we can play around a little with the particulars of how this is implemented before merging into the main branch. I'm targeting the 1.4.4 release for this since I think it's going to take some trial-and-error to avoid breaking the RPM build (the 1st-class citizen, so to speak) and the GitHub source tree build (what all the kool kids are doing).

mej added a commit that referenced this pull request Aug 17, 2020
Not all Linux distributions use "/usr/libexec" as it is marked
optional by the FHS.  This adds checks to the `nhcmain_init_env()`
function to make sure that NHC only uses directories that exist.

So now `$SYSCONFIGDIR` defaults to "/etc/sysconfig" ("/etc/default" on
Debian), but if that directory doesn't exist, NHC will fall back on
"/etc/default" if it exists or "/etc/nhc/sysconfig" if not.

Similarly, `$LIBEXECDIR` will fall back to "/usr/lib" if
"/usr/libexec" is not present.

This fixes #75, and it's highly likely to interfere with the ability
to merge #63 at a later date.
@mej mej modified the milestones: 1.4.4 Release, 1.4.4 Release (new), 1.5 Release Apr 17, 2021
@mej mej deleted the branch mej:bug/62-autoconf-paths March 9, 2022 19:02
@mej mej closed this Mar 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants