| Message ID | DB7PR10MB2508FAC53911DB211A98412AD2A50@DB7PR10MB2508.EURPRD10.PROD.OUTLOOK.COM |
|---|---|
| State | Under Review |
| Headers | show |
| Series | [pacman-dev] Use absolute path for pacman and pacman-conf in makepkg | expand |
On 8/5/20 4:13 am, Wouter Wijsman wrote: > Currently makepkg requires pacman and pacman-conf to be in the path of > the user. Since these executables should never move, it should be safe > to add the full paths to the scripts at compile time, assuming the user > uses the install command as well. > > This change works for both autotools and meson. > > Signed-off-by: Wouter Wijsman <wwijsman@live.nl> > --- Hi, Can you let us know more detail about the use case for this patch? Why would the user not add the directory pacman and scripts are installed in to their path? I have concerns that hardcoding paths on build will lead to difficulty when in the future we have a test suite for makepkg. Allan > build-aux/edit-script.sh.in | 1 + > meson.build | 1 + > scripts/Makefile.am | 3 ++- > scripts/makepkg.sh.in | 6 ++++-- > 4 files changed, 8 insertions(+), 3 deletions(-) > > diff --git a/build-aux/edit-script.sh.in b/build-aux/edit-script.sh.in > index 661c22d5..44103f08 100644 > --- a/build-aux/edit-script.sh.in > +++ b/build-aux/edit-script.sh.in > @@ -20,6 +20,7 @@ mode=$3 > -e "s|@DEBUGSUFFIX[@]|@DEBUGSUFFIX@|g" \ > -e "s|@INODECMD[@]|@INODECMD@|g" \ > -e "s|@FILECMD[@]|@FILECMD@|g" \ > + -e "s|@fullbindir[@]|@FULLBINDIR@|g" \ > "$input" >"$output" > > if [[ $mode ]]; then > diff --git a/meson.build b/meson.build > index 680cf62b..92e8a9a5 100644 > --- a/meson.build > +++ b/meson.build > @@ -257,6 +257,7 @@ substs.set('LOCALEDIR', LOCALEDIR) > substs.set('sysconfdir', SYSCONFDIR) > substs.set('localstatedir', LOCALSTATEDIR) > substs.set('PKGDATADIR', PKGDATADIR) > +substs.set('FULLBINDIR', '@0@/@1@'.format(PREFIX, get_option('bindir'))) > substs.set('PREFIX', PREFIX) > substs.set('BASH', BASH.path()) > substs.set('PACKAGE_VERSION', PACKAGE_VERSION) > diff --git a/scripts/Makefile.am b/scripts/Makefile.am > index 47455ed2..a79e9389 100644 > --- a/scripts/Makefile.am > +++ b/scripts/Makefile.am > @@ -182,7 +182,8 @@ edit = sed \ > -e 's|@DEBUGSUFFIX[@]|$(DEBUGSUFFIX)|g' \ > -e "s|@INODECMD[@]|$(INODECMD)|g" \ > -e "s|@FILECMD[@]|$(FILECMD)|g" \ > - -e 's|@SCRIPTNAME[@]|$@|g' > + -e 's|@SCRIPTNAME[@]|$@|g' \ > + -e 's|@fullbindir[@]|$(FULLBINDIR)|g' > > ## All the scripts depend on Makefile so that they are rebuilt when the > ## prefix etc. changes. Use chmod -w to prevent people from editing the > diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in > index d1416d15..2f7b838e 100644 > --- a/scripts/makepkg.sh.in > +++ b/scripts/makepkg.sh.in > @@ -44,6 +44,8 @@ declare -r makepkg_version='@PACKAGE_VERSION@' > declare -r confdir='@sysconfdir@' > declare -r BUILDSCRIPT='@BUILDSCRIPT@' > declare -r startdir="$(pwd -P)" > +declare -r PACMAN_EXECUTABLE='@fullbindir@/pacman' > +declare -r PACMAN_CONF_EXECUTABLE='@fullbindir@/pacman-conf' > > LIBRARY=${LIBRARY:-'@libmakepkgdir@'} > > @@ -235,7 +237,7 @@ run_pacman() { > else > cmd=(su root -c "$(printf '%q ' "${cmd[@]}")") > fi > - local lockfile="$(pacman-conf DBPath)/db.lck" > + local lockfile="$(${PACMAN_CONF_EXECUTABLE} DBPath)/db.lck" > while [[ -f $lockfile ]]; do > local timer=0 > msg "$(gettext "Pacman is currently in use, please wait...")" > @@ -1123,7 +1125,7 @@ unset var > PACKAGER=${PACKAGER:-"Unknown Packager"} > > # set pacman command if not already defined > -PACMAN=${PACMAN:-pacman} > +PACMAN=${PACMAN:-${PACMAN_EXECUTABLE}} > # save full path to command as PATH may change when sourcing /etc/profile > PACMAN_PATH=$(type -P $PACMAN) > >
On Mon, 2020-05-11 at 12:42 +1000, Allan McRae wrote: > On 8/5/20 4:13 am, Wouter Wijsman wrote: > > Currently makepkg requires pacman and pacman-conf to be in the path > > of > > the user. Since these executables should never move, it should be > > safe > > to add the full paths to the scripts at compile time, assuming the > > user > > uses the install command as well. > > > > This change works for both autotools and meson. > > > > Signed-off-by: Wouter Wijsman <wwijsman@live.nl> > > --- > > Hi, > > Can you let us know more detail about the use case for this > patch? Why > would the user not add the directory pacman and scripts are installed > in > to their path? > > I have concerns that hardcoding paths on build will lead to > difficulty > when in the future we have a test suite for makepkg. > > Allan Hi, I've since realized that this patch is not really needed. It was supposed to go together with another patch which didn't end up working. Sorry for the spam. I'm working on making pacman manage libraries for the Playstation Portable in a homebrew PSP software development kit. For this to work on both systems which already have pacman and systems which do not, I initially tried to make pacman build with different binary names. This approach had many issues and would require forking, which I'm not keen on doing. The Playstation Vita homebrew SDK and the devkit pro (multiple systems, mostly Nintendo) also use pacman, but they have these forks which are old and have some bugs not found in upstream pacman. Now I'm working on a different solution, which is to change the bindir in the meson options while building (to keep the binaries out of the path of the user) and using wrapper scripts for pacman and makepkg. Now I realize that I can just set the path in the wrapper script and be done with the issue this patch was supposed to solve. Right now an unpatched pacman almost works for my use case, but there still is one issue with building it. Currently ninja will install the bash-completion scripts system wide, regardless of which user runs ninja install or what the prefix is. Only if bash-completion is not available, will the scripts be installed inside the prefix. The code which makes this happen is here: https://git.archlinux.org/pacman.git/tree/meson.build#n46 This is why I submitted the patch which makes bash-completion optional. It may need a different solution, though. Would you take a patch which adds an option to install the bash-completion scripts inside the prefix? I'd prefer not to maintain any private patches if that's possible. Kind regards, Wouter P.S. For the people interested in the wrapper, here is a link: https://github.com/sharkwouter/psp-pacman-1/tree/upstream-pr The pacman.sh script builds and installs pacman. The wrapper scripts are in scripts. If the user already has pacman, it doesn't build it at all and the wrapper will use the system's pacman with a different configuration, root and dbpath.
On 5/11/20 9:01 AM, wwijsman@live.nl wrote: > Right now an unpatched pacman almost works for my use case, but there > still is one issue with building it. Currently ninja will install the > bash-completion scripts system wide, regardless of which user runs > ninja install or what the prefix is. Only if bash-completion is not > available, will the scripts be installed inside the prefix. The code > which makes this happen is here: > https://git.archlinux.org/pacman.git/tree/meson.build#n46 > > This is why I submitted the patch which makes bash-completion optional. > It may need a different solution, though. Would you take a patch which > adds an option to install the bash-completion scripts inside the > prefix? I'd prefer not to maintain any private patches if that's > possible. https://git.archlinux.org/users/eschwartz/pacman.git/commit/?h=queue2&id=b4e3e1c7d93463dffdbb63ff78008df0f57780db It's simple enough to do, I've just been trying to decide whether I think it is the right solution before proposing it.
diff --git a/build-aux/edit-script.sh.in b/build-aux/edit-script.sh.in index 661c22d5..44103f08 100644 --- a/build-aux/edit-script.sh.in +++ b/build-aux/edit-script.sh.in @@ -20,6 +20,7 @@ mode=$3 -e "s|@DEBUGSUFFIX[@]|@DEBUGSUFFIX@|g" \ -e "s|@INODECMD[@]|@INODECMD@|g" \ -e "s|@FILECMD[@]|@FILECMD@|g" \ + -e "s|@fullbindir[@]|@FULLBINDIR@|g" \ "$input" >"$output" if [[ $mode ]]; then diff --git a/meson.build b/meson.build index 680cf62b..92e8a9a5 100644 --- a/meson.build +++ b/meson.build @@ -257,6 +257,7 @@ substs.set('LOCALEDIR', LOCALEDIR) substs.set('sysconfdir', SYSCONFDIR) substs.set('localstatedir', LOCALSTATEDIR) substs.set('PKGDATADIR', PKGDATADIR) +substs.set('FULLBINDIR', '@0@/@1@'.format(PREFIX, get_option('bindir'))) substs.set('PREFIX', PREFIX) substs.set('BASH', BASH.path()) substs.set('PACKAGE_VERSION', PACKAGE_VERSION) diff --git a/scripts/Makefile.am b/scripts/Makefile.am index 47455ed2..a79e9389 100644 --- a/scripts/Makefile.am +++ b/scripts/Makefile.am @@ -182,7 +182,8 @@ edit = sed \ -e 's|@DEBUGSUFFIX[@]|$(DEBUGSUFFIX)|g' \ -e "s|@INODECMD[@]|$(INODECMD)|g" \ -e "s|@FILECMD[@]|$(FILECMD)|g" \ - -e 's|@SCRIPTNAME[@]|$@|g' + -e 's|@SCRIPTNAME[@]|$@|g' \ + -e 's|@fullbindir[@]|$(FULLBINDIR)|g' ## All the scripts depend on Makefile so that they are rebuilt when the ## prefix etc. changes. Use chmod -w to prevent people from editing the diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index d1416d15..2f7b838e 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -44,6 +44,8 @@ declare -r makepkg_version='@PACKAGE_VERSION@' declare -r confdir='@sysconfdir@' declare -r BUILDSCRIPT='@BUILDSCRIPT@' declare -r startdir="$(pwd -P)" +declare -r PACMAN_EXECUTABLE='@fullbindir@/pacman' +declare -r PACMAN_CONF_EXECUTABLE='@fullbindir@/pacman-conf' LIBRARY=${LIBRARY:-'@libmakepkgdir@'} @@ -235,7 +237,7 @@ run_pacman() { else cmd=(su root -c "$(printf '%q ' "${cmd[@]}")") fi - local lockfile="$(pacman-conf DBPath)/db.lck" + local lockfile="$(${PACMAN_CONF_EXECUTABLE} DBPath)/db.lck" while [[ -f $lockfile ]]; do local timer=0 msg "$(gettext "Pacman is currently in use, please wait...")" @@ -1123,7 +1125,7 @@ unset var PACKAGER=${PACKAGER:-"Unknown Packager"} # set pacman command if not already defined -PACMAN=${PACMAN:-pacman} +PACMAN=${PACMAN:-${PACMAN_EXECUTABLE}} # save full path to command as PATH may change when sourcing /etc/profile PACMAN_PATH=$(type -P $PACMAN)
Currently makepkg requires pacman and pacman-conf to be in the path of the user. Since these executables should never move, it should be safe to add the full paths to the scripts at compile time, assuming the user uses the install command as well. This change works for both autotools and meson. Signed-off-by: Wouter Wijsman <wwijsman@live.nl> --- build-aux/edit-script.sh.in | 1 + meson.build | 1 + scripts/Makefile.am | 3 ++- scripts/makepkg.sh.in | 6 ++++-- 4 files changed, 8 insertions(+), 3 deletions(-)