[pacman-dev] [PATCH] makepkg: do not run prepare() with --noextract
This is partial revert of 8454daa7feef (makepkg: run pkgver() and prepare() with --noextract). Reasoning for the reversion (copied from FS#43498): Running prepare() when --noextract is used no longer allows running 'makepkg -o && makepkg -e' with any PKGBUILD that applies patches in prepare(). [1] Sure there's --noprepare which restores the old behavior, but that's a lot of extra typing for what I believe is a much more common use of --noextract. For OP's use case of doing git bisects, you can specify the commit in the source array and thus skip --noextract since makepkg will checkout the correct commit each time. [1] I often extract the sources using 'makepkg -o', manually edit some source files, and then use 'makepkg -e' to package it (while possibly repeating the edit/package steps). --- doc/makepkg.8.txt | 10 +++++----- scripts/makepkg.sh.in | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/doc/makepkg.8.txt b/doc/makepkg.8.txt index 6f3f6b0..2dff1b1 100644 --- a/doc/makepkg.8.txt +++ b/doc/makepkg.8.txt @@ -55,11 +55,11 @@ Options installed. *-e, \--noextract*:: - Do not extract source files; use whatever source already exists in the - $srcdir/ directory. This is handy if you want to go into $srcdir/ and - manually patch or tweak code, then make a package out of the result. - Keep in mind that creating a patch may be a better solution to allow - others to use your PKGBUILD. + Do not extract source files or run the prepare() function (if present); + use whatever source already exists in the $srcdir/ directory. This is + handy if you want to go into $srcdir/ and manually patch or tweak code, + then make a package out of the result. Keep in mind that creating a + patch may be a better solution to allow others to use your PKGBUILD. *\--verifysource*:: For each source file in the source array of PKGBUILD, download the file diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 80b95f4..2fa3058 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -2206,6 +2206,9 @@ if (( !REPKG )); then fi extract_sources + if (( PREPAREFUNC )); then + run_prepare + fi fi if (( PKGVERFUNC )); then @@ -2213,9 +2216,6 @@ if (( !REPKG )); then basever=$(get_full_version) check_build_status fi - if (( PREPAREFUNC )); then - run_prepare - fi fi if (( NOBUILD )); then -- 2.7.1
On 17/02/16 17:11, Evangelos Foutras wrote:
This is partial revert of 8454daa7feef (makepkg: run pkgver() and prepare() with --noextract).
Reasoning for the reversion (copied from FS#43498):
Running prepare() when --noextract is used no longer allows running 'makepkg -o && makepkg -e' with any PKGBUILD that applies patches in prepare(). [1]
Sure there's --noprepare which restores the old behavior, but that's a lot of extra typing for what I believe is a much more common use of --noextract.
So you are taking this from a case where you can use makepkg --noextract with or without prepare() and making it so you can only use it without to avoid some typing...
On 17/02/16 09:50, Allan McRae wrote:
On 17/02/16 17:11, Evangelos Foutras wrote:
This is partial revert of 8454daa7feef (makepkg: run pkgver() and prepare() with --noextract).
Reasoning for the reversion (copied from FS#43498):
Running prepare() when --noextract is used no longer allows running 'makepkg -o && makepkg -e' with any PKGBUILD that applies patches in prepare(). [1]
Sure there's --noprepare which restores the old behavior, but that's a lot of extra typing for what I believe is a much more common use of --noextract.
So you are taking this from a case where you can use makepkg --noextract with or without prepare() and making it so you can only use it without to avoid some typing...
When there is no reason to be doing that, then removing this "feature" is the correct approach. prepare() is there to modify the sources after extraction; otherwise we might as well move its body into build(). Quoting another of my comments on FS#43498: ===================================================================== I don't see the need for a --prepare switch to be used together with --noextract (I consider prepare() tightly coupled to source extraction); its usefulness would be minimal at best. If someone really wants to call prepare() when not extracting sources (for some special testing/bisect scenario), they can call it from build(). ===================================================================== Running prepare() with --noextract makes no sense to me and, therefore, do not believe it needs to be a supported configuration. YAGNI seems to apply in this case. [1] [1] https://en.wikipedia.org/wiki/You_aren't_gonna_need_it
I kind of agree with this and would rather have a prepare switch to turn it on with no extract. Most of the time I have a prepare, it is for patching the source, and if I am not extracting, then the patching would fail because the source isn't in the right place. My $0.02 Daniel On Thu, Feb 18, 2016 at 8:44 PM Evangelos Foutras <evangelos@foutrelis.com> wrote:
On 17/02/16 09:50, Allan McRae wrote:
On 17/02/16 17:11, Evangelos Foutras wrote:
This is partial revert of 8454daa7feef (makepkg: run pkgver() and prepare() with --noextract).
Reasoning for the reversion (copied from FS#43498):
Running prepare() when --noextract is used no longer allows running 'makepkg -o && makepkg -e' with any PKGBUILD that applies patches in prepare(). [1]
Sure there's --noprepare which restores the old behavior, but that's a lot of extra typing for what I believe is a much more common use of --noextract.
So you are taking this from a case where you can use makepkg --noextract with or without prepare() and making it so you can only use it without to avoid some typing...
When there is no reason to be doing that, then removing this "feature" is the correct approach. prepare() is there to modify the sources after extraction; otherwise we might as well move its body into build().
Quoting another of my comments on FS#43498:
===================================================================== I don't see the need for a --prepare switch to be used together with --noextract (I consider prepare() tightly coupled to source extraction); its usefulness would be minimal at best.
If someone really wants to call prepare() when not extracting sources (for some special testing/bisect scenario), they can call it from build(). =====================================================================
Running prepare() with --noextract makes no sense to me and, therefore, do not believe it needs to be a supported configuration. YAGNI seems to apply in this case. [1]
On Fri, Feb 19, 2016 at 4:54 AM, Daniel Wallace <danielwallace@gtmanfred.com> wrote:
I kind of agree with this and would rather have a prepare switch to turn it on with no extract.
That appears to be Allan's suggestion more or less; restoring the old behavior but also providing a --prepare option to allow running prepare() with --noextract. I'm advocating that such an option has a tiny or even nonexistent use case, to the point that it should not be implemented at all; i.e.: it would be inelegant to have such an option.
The core questions seems to be whether prepare() is part of the source download and extraction process. In my conception, it is. I also normally use prepare() to patch or modify sources, which usually fails without prior extraction. So I would second to not execute prepare() when --noextract is given, but also do execute prepare() when -o / --nobuild is given. Though I doubt many would have the need to, options to restore the current behavior (--prepare & --noprepare) should be provided for everyone depending on it.
On 19/02/16 13:23, Evangelos Foutras wrote:
On Fri, Feb 19, 2016 at 4:54 AM, Daniel Wallace <danielwallace@gtmanfred.com> wrote:
I kind of agree with this and would rather have a prepare switch to turn it on with no extract.
That appears to be Allan's suggestion more or less; restoring the old behavior but also providing a --prepare option to allow running prepare() with --noextract. I'm advocating that such an option has a tiny or even nonexistent use case, to the point that it should not be implemented at all; i.e.: it would be inelegant to have such an option.
I'm going to pull this patch. But if a good use-case for running prepare() with --noextract is presented, either you need to provide a patch providing a --prepare flag, or this will be reverted. Allan
I just noticed that this also reverses the order of the functions. In pacman 5.0.0, pkgver still ran before prepare. It indeed makes more sense to me to determine the pkgver from the patched or otherwise fixed sources. The change simplifies many of my packages. However, it is not mentioned in the commit message. Is this intentional? Will it stay this way?
On 06/03/16 04:19, Dominik Fischer wrote:
I just noticed that this also reverses the order of the functions. In pacman 5.0.0, pkgver still ran before prepare.
It indeed makes more sense to me to determine the pkgver from the patched or otherwise fixed sources. The change simplifies many of my packages. However, it is not mentioned in the commit message.
Is this intentional? Will it stay this way?
It was unintended with that patch, but had been intended to happen in the next release... It will stay that way and I will update the documentation. A
participants (4)
-
Allan McRae
-
Daniel Wallace
-
Dominik Fischer
-
Evangelos Foutras