[pacman-dev] [patch] add support for scriptlets functions embedded in $BUILDSCRIPT
Hi, I think there isn't a real need to split PKGBUILD and $pkgname.install, this patch allows you to put the scriptlets functions into the PKGBUILD. This IMHO follows the KISS philosophy. If at least a {post,pre}_{install,upgrade,remove} function is found in the PKGBUILD, then the install file (if it exists) is ignored; otherwise, if there isn't any scriptlet function in the PKGBUILD the install file is included, as usually. Enjoy: From e206a96ebe1e9a11f85de1502fc46e75321ba192 Mon Sep 17 00:00:00 2001 From: Alessio 'mOLOk' Bolognino <themolok@gmail.com> Date: Sat, 21 Jul 2007 06:15:34 +0200 Subject: [PATCH] Added support for scriptlets functions embedded in $BUILDSCRIPT --- scripts/makepkg.sh.in | 14 ++++++++++++-- 1 files changed, 12 insertions(+), 2 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index a3e4d9c..fb931a7 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -831,9 +831,18 @@ create_package() { local comp_files=".PKGINFO .FILELIST" - # check for an install script + # check for scriptlets fuctions or an install script # TODO: should we include ${pkgname}.install if it exists and $install is unset? - if [ "$install" != "" ]; then + scriptlet=$(declare -f {post,pre}_{install,remove,upgrade} || return 0) + if [ "$scriptlet" != "" ]; then + msg2 "$(gettext "Adding scriptlet functions (install script will be ignored)...")" + ( echo "$scriptlet" + echo 'op=$1' + echo 'shift' + echo '[ "$(type -t "$op")" = "function" ] && $op "$@"' + ) > .INSTALL + comp_files="$comp_files .INSTALL" + elif [ "$install" != "" ]; then msg2 "$(gettext "Adding install script...")" cp "$startdir/$install" .INSTALL comp_files="$comp_files .INSTALL" @@ -1175,6 +1184,7 @@ fi unset pkgname pkgver pkgrel pkgdesc url license groups provides md5sums force unset replaces depends conflicts backup source install build makedepends unset options noextract +unset {post,pre}_{install,remove,upgrade} if [ ! -f "$BUILDSCRIPT" ]; then error "$(gettext "%s does not exist.")" "$BUILDSCRIPT" -- 1.5.2.4 -- Alessio 'mOLOk' Bolognino Arch Linux Trusted User Please send personal email to themolok@gmail.com Public Key http://pgp.mit.edu:11371/pks/lookup?op=get&search=0xFE0270FB GPG Key ID = 1024D / FE0270FB 2007-04-11 Key Fingerprint = 9AF8 9011 F271 450D 59CF 2D7D 96C9 8F2A FE02 70FB
On Sat 2007-07-21 06:28 , Alessio 'mOLOk' Bolognino wrote:
Hi, I think there isn't a real need to split PKGBUILD and $pkgname.install, this patch allows you to put the scriptlets functions into the PKGBUILD. This IMHO follows the KISS philosophy. If at least a {post,pre}_{install,upgrade,remove} function is found in the PKGBUILD, then the install file (if it exists) is ignored; otherwise, if there isn't any scriptlet function in the PKGBUILD the install file is included, as usually. Enjoy:
Ping? A "Won't merge because..." would be appreciated too :) -- Alessio 'mOLOk' Bolognino Arch Linux Trusted User Please send personal email to themolok@gmail.com Public Key http://pgp.mit.edu:11371/pks/lookup?op=get&search=0xFE0270FB GPG Key ID = 1024D / FE0270FB 2007-04-11 Key Fingerprint = 9AF8 9011 F271 450D 59CF 2D7D 96C9 8F2A FE02 70FB
2007/8/2, Alessio 'mOLOk' Bolognino <themolok.ml@gmail.com>:
On Sat 2007-07-21 06:28 , Alessio 'mOLOk' Bolognino wrote:
Hi, I think there isn't a real need to split PKGBUILD and $pkgname.install, this patch allows you to put the scriptlets functions into the PKGBUILD. This IMHO follows the KISS philosophy. If at least a {post,pre}_{install,upgrade,remove} function is found in the PKGBUILD, then the install file (if it exists) is ignored; otherwise, if there isn't any scriptlet function in the PKGBUILD the install file is included, as usually. Enjoy:
Ping? A "Won't merge because..." would be appreciated too :)
I guess this mail just lose in everyone's mail queue. :P Ok, I must confess I've read it but then forgot. The overall idea seems good for me. I don't see any pitfalls in merging .install into PKGBUILD. BTW, Dan will be back after 12th. -- Roman Kyrylych (Роман Кирилич)
On 8/1/07, Alessio 'mOLOk' Bolognino <themolok.ml@gmail.com> wrote:
Ping? A "Won't merge because..." would be appreciated too :)
Sorry sorry. Blame me, I've been slacking in my duties for too long. I actually think I like this. Have you tested it? I just want to verify that it is backwards compatible and all that. Also: 'declare' - man, I have been wondering about that forever and just never looked into it. Thanks for this. Does anyone have a problem with this? I think merging the files could be beneficial in the long run.
Hello, Na Thu, Aug 02, 2007 at 05:12:30PM -0500, Aaron Griffin <aaronmgriffin@gmail.com> pisal(a):
Does anyone have a problem with this? I think merging the files could be beneficial in the long run.
what do you plan to do with the pkgs seding the install file in build()? just keep them as is? - VMiklos
On Fri, 3 Aug 2007 00:39:38 +0200 VMiklos <vmiklos@frugalware.org> wrote:
Hello,
Na Thu, Aug 02, 2007 at 05:12:30PM -0500, Aaron Griffin <aaronmgriffin@gmail.com> pisal(a):
Does anyone have a problem with this? I think merging the files could be beneficial in the long run.
what do you plan to do with the pkgs seding the install file in build()?
just keep them as is?
- VMiklos
Along those lines, what about functions that reference a global variable inside the PKGBUILD? like: __KERNEL_VERSION=2.6.12 post_install() { depmod -v $__KERNEL_VERSION > /dev/null 2>&1 } I could see that specifically happening in kernel modules (which is where a lot of the .install file sed-ding happens, and why I remembered). If those variables aren't there in the .INSTALL file, then what? -- Travis
On 8/2/07, Travis Willard <travis@archlinux.org> wrote:
Along those lines, what about functions that reference a global variable inside the PKGBUILD? like:
__KERNEL_VERSION=2.6.12
post_install() { depmod -v $__KERNEL_VERSION > /dev/null 2>&1 }
I could see that specifically happening in kernel modules (which is where a lot of the .install file sed-ding happens, and why I remembered). If those variables aren't there in the .INSTALL file, then what?
Oh good point, I didn't think that far ahead. if that's the case, then we have two options: a) this patch doesn't force usage one way or the other, it just _allows_ things to be inline. Kernel modules can still be kept external b) Alessio, can you think of a way to overcome this issue?
On Thu 2007-08-02 17:12 , Aaron Griffin wrote:
On 8/1/07, Alessio 'mOLOk' Bolognino <themolok.ml@gmail.com> wrote:
Ping? A "Won't merge because..." would be appreciated too :)
Sorry sorry. Blame me, I've been slacking in my duties for too long.
I actually think I like this. Have you tested it? I just want to verify that it is backwards compatible and all that.
I tested it on few packages, it is backwards compatible because it checks for the existence of a .install file too, BUT if you simply concatenate the PKGBUILD and the .install, then it could not work because of shared variables between the .install functions and others particular operation on the .install file made by the build function (as Travis and VMiklos pointed out). The variables like KERNEL_VERSION should be added in all {post,pre}_{install,upgrade,remove} function and I admit this is not the best way and not even very intuitive, but I can't see any non-hackish way to handle it. -- Alessio 'mOLOk' Bolognino Arch Linux Trusted User Please send personal email to themolok@gmail.com Public Key http://pgp.mit.edu:11371/pks/lookup?op=get&search=0xFE0270FB GPG Key ID = 1024D / FE0270FB 2007-04-11 Key Fingerprint = 9AF8 9011 F271 450D 59CF 2D7D 96C9 8F2A FE02 70FB
On Fri 2007-08-03 02:34 , Alessio 'mOLOk' Bolognino wrote:
On Thu 2007-08-02 17:12 , Aaron Griffin wrote:
On 8/1/07, Alessio 'mOLOk' Bolognino <themolok.ml@gmail.com> wrote:
Ping? A "Won't merge because..." would be appreciated too :)
Sorry sorry. Blame me, I've been slacking in my duties for too long.
I actually think I like this. Have you tested it? I just want to verify that it is backwards compatible and all that.
I tested it on few packages, it is backwards compatible because it checks for the existence of a .install file too, BUT if you simply concatenate the PKGBUILD and the .install, then it could not work because of shared variables between the .install functions and others particular operation on the .install file made by the build function (as Travis and VMiklos pointed out).
s/shared variables/global variables/ -- Alessio 'mOLOk' Bolognino Arch Linux Trusted User Please send personal email to themolok@gmail.com Public Key http://pgp.mit.edu:11371/pks/lookup?op=get&search=0xFE0270FB GPG Key ID = 1024D / FE0270FB 2007-04-11 Key Fingerprint = 9AF8 9011 F271 450D 59CF 2D7D 96C9 8F2A FE02 70FB
On Fri 2007-08-03 02:34 , Alessio 'mOLOk' Bolognino wrote:
On Thu 2007-08-02 17:12 , Aaron Griffin wrote:
On 8/1/07, Alessio 'mOLOk' Bolognino <themolok.ml@gmail.com> wrote:
Ping? A "Won't merge because..." would be appreciated too :)
Sorry sorry. Blame me, I've been slacking in my duties for too long.
I actually think I like this. Have you tested it? I just want to verify that it is backwards compatible and all that.
I tested it on few packages, it is backwards compatible because it checks for the existence of a .install file too, BUT if you simply concatenate the PKGBUILD and the .install, then it could not work because of shared variables between the .install functions and others particular operation on the .install file made by the build function (as Travis and VMiklos pointed out).
The variables like KERNEL_VERSION should be added in all {post,pre}_{install,upgrade,remove} function and I admit this is not the best way and not even very intuitive, but I can't see any non-hackish way to handle it. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ But I can code an hackish way to do it! :) With this patch, makepkg checks which global variables are used in the the scriptlets and add them on the top of the .INSTALL file. Works fine for variables used like:
$variable ${variable} ${variable/foo/bar} ${variable##foo} ${variable%%bar} but DOESN'T work with "complex" operations such as $(( variable1 + variable2 )) This is a limitation, might be confusing, and have unexpected behaviour but probably no one will ever need to do arithmetic operations with global variables. This patch lets you use PKGBUILD variables like $pkgname $license etc. in the {post,pre}_{upgrade,remove,install} functions. Is it useful? Probably it is not, but you _can_ do it, you don't _have_to_ do it :) Enjoy: --- scripts/makepkg.sh.in | 28 +++++++++++++++++++++++++--- 1 files changed, 25 insertions(+), 3 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index a3e4d9c..415ac00 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -830,10 +830,31 @@ create_package() { fi local comp_files=".PKGINFO .FILELIST" - - # check for an install script + # check for scriptlets fuctions or an install script # TODO: should we include ${pkgname}.install if it exists and $install is unset? - if [ "$install" != "" ]; then + local scriptlet="$(declare -f {post,pre}_{install,remove,upgrade} || return 0)" + if [ "$scriptlet" != "" ]; then + # check global variables used by scriptlet functions + local scr_vars="$(echo $scriptlet \ + | egrep -o '\$\{?[[:alpha:]_][[:alnum:]_#%/-]*\}?' \ + | sed -e 's@[#%/].*@@g' -e 's@[{}$]@@g' | sort -u)" + + for scr_var in $scr_vars; do + local glob_var=$( set | egrep "^${scr_var}=" || return 0) + if [[ "$glob_var" != "" ]]; then + local glob_vars=$(echo -e "${glob_vars}\n${glob_var}") + fi + done + + msg2 "$(gettext "Adding scriptlet functions (install script will be ignored)...")" + ( echo "$glob_vars" + echo "$scriptlet" + echo 'op=$1' + echo 'shift' + echo '[ "$(type -t "$op")" = "function" ] && $op "$@"' + ) > .INSTALL + comp_files="$comp_files .INSTALL" + elif [ "$install" != "" ]; then msg2 "$(gettext "Adding install script...")" cp "$startdir/$install" .INSTALL comp_files="$comp_files .INSTALL" @@ -1175,6 +1196,7 @@ fi unset pkgname pkgver pkgrel pkgdesc url license groups provides md5sums force unset replaces depends conflicts backup source install build makedepends unset options noextract +unset {post,pre}_{install,remove,upgrade} if [ ! -f "$BUILDSCRIPT" ]; then error "$(gettext "%s does not exist.")" "$BUILDSCRIPT" -- 1.5.2.4 -- Alessio 'mOLOk' Bolognino Arch Linux Trusted User Please send personal email to themolok@gmail.com Public Key http://pgp.mit.edu:11371/pks/lookup?op=get&search=0xFE0270FB GPG Key ID = 1024D / FE0270FB 2007-04-11 Key Fingerprint = 9AF8 9011 F271 450D 59CF 2D7D 96C9 8F2A FE02 70FB
I really like the idea of a single file build script. I must admit, it comparison, it does seem a bit odd to have {post,pre} install operations present in an additional file, when compared to a 'fully-integrated' pkgbuild.
On 8/3/07, Alessio 'mOLOk' Bolognino <themolok.ml@gmail.com> wrote:
On Fri 2007-08-03 02:34 , Alessio 'mOLOk' Bolognino wrote:
but I can't see any non-hackish way to handle it. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ But I can code an hackish way to do it! :)
Ok, one suggestion and I think I'll be ok with it - can you do two things as part of this patch: a) document that complex variable usage will fail in the man page b) Output a warning if variables are found - something that says "don't do crazy stuff!" Assuming that's all in there (so you get credit), I don't think I'd have a problem with merging this - anyone else?
On Fri 2007-08-03 15:18 , Aaron Griffin wrote:
On 8/3/07, Alessio 'mOLOk' Bolognino <themolok.ml@gmail.com> wrote:
On Fri 2007-08-03 02:34 , Alessio 'mOLOk' Bolognino wrote:
but I can't see any non-hackish way to handle it. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ But I can code an hackish way to do it! :)
Ok, one suggestion and I think I'll be ok with it - can you do two things as part of this patch: a) document that complex variable usage will fail in the man page b) Output a warning if variables are found - something that says "don't do crazy stuff!"
Assuming that's all in there (so you get credit), I don't think I'd have a problem with merging this - anyone else?
Uhm... this took a while, I almost forgot about this :) Here it is my patch, all critics are welcome: --- doc/PKGBUILD.5 | 10 ++++++++-- scripts/makepkg.sh.in | 34 +++++++++++++++++++++++++++++++--- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/doc/PKGBUILD.5 b/doc/PKGBUILD.5 index 747b9f1..f69f1c0 100644 --- a/doc/PKGBUILD.5 +++ b/doc/PKGBUILD.5 @@ -220,8 +220,8 @@ script is run right before files are removed. script is run right after files are removed. .P -To use this feature, create a file such as 'pkgname.install' and put it in -the same directory as the \fB\*(PB\fP script. Then use the \fBinstall\fP +To use this feature, you can create a file such as 'pkgname.install' and put it +in the same directory as the \fB\*(PB\fP script. Then use the \fBinstall\fP directive: .RS @@ -233,6 +233,12 @@ install=pkgname.install The install script does not need to be specified in the \fBsource\fP array. A template install file is available in the ABS tree (/var/abs/install.proto). +You can also embed the scripts in the \fB\*(PB\fP. In this case you can use +global variables (such as pkgver, pkgname, etc.) inside the scripts, but this +is deprecated if not strictly needed. Note that the usage of global +variables in arithmetical expansion expressions or in other complex expressions +will likely fail. + .SH EXAMPLE The following is an example \fB\*(PB\fP for the 'modutils' package. For more examples, look through the ABS tree. diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index cd5dbf5..37ca87f 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -833,10 +833,37 @@ create_package() { fi local comp_files=".PKGINFO .FILELIST" - - # check for an install script + # check for scriptlets fuctions or an install script # TODO: should we include ${pkgname}.install if it exists and $install is unset? - if [ "$install" != "" ]; then + local scriptlet="$(declare -f {post,pre}_{install,remove,upgrade} || return 0)" + if [ "$scriptlet" != "" ]; then + # check global variables used by scriptlet functions + local scr_vars="$(echo $scriptlet \ + | egrep -o '\$\{?[[:alpha:]_][[:alnum:]_#%/-]*\}?' \ + | sed -e 's@[#%/].*@@g' -e 's@[{}$]@@g' | sort -u)" + + for scr_var in $scr_vars; do + local glob_var=$( set | egrep "^${scr_var}=" || return 0) + if [[ "$glob_var" != "" ]]; then + local glob_vars=$(echo -e "${glob_vars}\n${glob_var}") + fi + done + + msg2 "$(gettext "Adding scriptlet functions (install script will be ignored)...")" + ( echo "$glob_vars" + echo "$scriptlet" + echo 'op=$1' + echo 'shift' + echo '[ "$(type -t "$op")" = "function" ] && $op "$@"' + ) > .INSTALL + + if [[ "$glob_vars" != "" ]]; then + msg2 "$(gettext "Global variables usage in scriptlets found")" + msg2 "$(gettext "NOTE: this is deprecated if not strictly needed")" + fi + + comp_files="$comp_files .INSTALL" + elif [ "$install" != "" ]; then msg2 "$(gettext "Adding install script...")" cp "$startdir/$install" .INSTALL comp_files="$comp_files .INSTALL" @@ -1178,6 +1205,7 @@ fi unset pkgname pkgver pkgrel pkgdesc url license groups provides md5sums force unset replaces depends conflicts backup source install build makedepends unset options noextract +unset {post,pre}_{install,remove,upgrade} if [ ! -f "$BUILDSCRIPT" ]; then error "$(gettext "%s does not exist.")" "$BUILDSCRIPT" -- 1.5.3 -- Alessio 'mOLOk' Bolognino Arch Linux Trusted User Please send personal email to themolok@gmail.com Public Key http://pgp.mit.edu:11371/pks/lookup?op=get&search=0xFE0270FB GPG Key ID = 1024D / FE0270FB 2007-04-11 Key Fingerprint = 9AF8 9011 F271 450D 59CF 2D7D 96C9 8F2A FE02 70FB
I'm just going to poke this thread here. With a bug reference: http://bugs.archlinux.org/task/3244 I think this is totally feasible. Guys?
On Jan 23, 2008 4:34 PM, Aaron Griffin <aaronmgriffin@gmail.com> wrote:
I'm just going to poke this thread here. With a bug reference: http://bugs.archlinux.org/task/3244
I think this is totally feasible. Guys?
Oh wow, I thought this has already been merged. Yeah, go for it - I like it.
2008/1/23, Aaron Griffin <aaronmgriffin@gmail.com>:
I'm just going to poke this thread here. With a bug reference: http://bugs.archlinux.org/task/3244
I think this is totally feasible. Guys?
Wow, I've totally forgot about this. What can I say?.. Apply it and let us test how cool it is. :-D -- Roman Kyrylych (Роман Кирилич)
On Jan 23, 2008 3:42 PM, Roman Kyrylych <roman.kyrylych@gmail.com> wrote:
2008/1/23, Aaron Griffin <aaronmgriffin@gmail.com>:
I'm just going to poke this thread here. With a bug reference: http://bugs.archlinux.org/task/3244
I think this is totally feasible. Guys?
Wow, I've totally forgot about this. What can I say?.. Apply it and let us test how cool it is. :-D
I'm going to be the hardass here and first ask which patch we are considering above. The global variables thing worries me a bit. I feel like the advantages of having it in a second file, which is copied directly to the package, outweighs the benefits. Notice that with the above method, zero comments are copied over as well. This could confuse people. I already mentioned the global variables thing as a drawback, in addition to using subfunctions (see the pacman.install file). It isn't that these couldn't be solved by simply sticking with an external install file, but the confusion of educating users then comes into play. "Hey, you can put install functions in your PKGBUILD, but ONLY if they don't use variables, you don't mind missing comments, you don't try to actually use subfunctions to clean up the code...". Maybe I'm just being a stickler here, but it seems like what we have now works quite well, at the expense of needing one extra file. It also *clearly* seperates build-time operations from install-time ones, which I can see being quite confusing to first-time PKGBUILD writers. -Dan
I'm who wrote the patch, so I think I should protect my baby a little bit, even if I am not an strong supporter of it: On Wed 2008-01-23 17:45 , Dan McGee wrote:
[...] I'm going to be the hardass here and first ask which patch we are considering above. The global variables thing worries me a bit.
Global variables work 90% of time, they don't work if you mess with them too much (and it's documented)
I feel like the advantages of having it in a second file, which is copied directly to the package, outweighs the benefits. Notice that with the above method, zero comments are copied over as well.
True, but usually people look at the source PKGBUILD, not at /var/lib/pacman/wherever/is/the/post-install
This could confuse people. I already mentioned the global variables thing as a drawback, in addition to using subfunctions (see the pacman.install file). It isn't that these couldn't be solved by simply sticking with an external install file, but the confusion of educating users then comes into play. "Hey, you can put install functions in your PKGBUILD, but ONLY if they don't use variables, you don't mind missing comments, you don't try to actually use subfunctions to clean up the code...".
Maybe I'm just being a stickler here, but it seems like what we have now works quite well, at the expense of needing one extra file. It also *clearly* seperates build-time operations from install-time ones, which I can see being quite confusing to first-time PKGBUILD writers.
I agree that an external file is more straightforward, but as you said, the patch only adds a feature, it's fully backward compatible. I wrote this mail also to say that I'm not going to develop this patch anymore, so if it doesn't apply or makes your systems explode, don't ask me to fix it because I don't have time/will to do it. -- Alessio Bolognino Please send personal email to themolok@gmail.com Public Key http://pgp.mit.edu:11371/pks/lookup?op=get&search=0xFE0270FB GPG Key ID = 1024D / FE0270FB 2007-04-11 Key Fingerprint = 9AF8 9011 F271 450D 59CF 2D7D 96C9 8F2A FE02 70FB
2008/1/24, Alessio Bolognino <themolok.ml@gmail.com>:
I'm who wrote the patch, so I think I should protect my baby a little bit, even if I am not an strong supporter of it:
On Wed 2008-01-23 17:45 , Dan McGee wrote:
[...]
I'm going to be the hardass here and first ask which patch we are considering above. The global variables thing worries me a bit.
Global variables work 90% of time, they don't work if you mess with them too much (and it's documented)
I feel like the advantages of having it in a second file, which is copied directly to the package, outweighs the benefits. Notice that with the above method, zero comments are copied over as well.
True, but usually people look at the source PKGBUILD, not at /var/lib/pacman/wherever/is/the/post-install
This could confuse people. I already mentioned the global variables thing as a drawback, in addition to using subfunctions (see the pacman.install file). It isn't that these couldn't be solved by simply sticking with an external install file, but the confusion of educating users then comes into play. "Hey, you can put install functions in your PKGBUILD, but ONLY if they don't use variables, you don't mind missing comments, you don't try to actually use subfunctions to clean up the code...".
Maybe I'm just being a stickler here, but it seems like what we have now works quite well, at the expense of needing one extra file. It also *clearly* seperates build-time operations from install-time ones, which I can see being quite confusing to first-time PKGBUILD writers.
I agree that an external file is more straightforward, but as you said, the patch only adds a feature, it's fully backward compatible.
I wrote this mail also to say that I'm not going to develop this patch anymore, so if it doesn't apply or makes your systems explode, don't ask me to fix it because I don't have time/will to do it.
Trying to resurrect this... Dan, can this be merged for 3.2? -- Roman Kyrylych (Роман Кирилич)
On Mon, Feb 25, 2008 at 6:57 AM, Roman Kyrylych <roman.kyrylych@gmail.com> wrote:
2008/1/24, Alessio Bolognino <themolok.ml@gmail.com>:
I'm who wrote the patch, so I think I should protect my baby a little bit, even if I am not an strong supporter of it:
On Wed 2008-01-23 17:45 , Dan McGee wrote:
[...]
I'm going to be the hardass here and first ask which patch we are considering above. The global variables thing worries me a bit.
Global variables work 90% of time, they don't work if you mess with them too much (and it's documented)
I feel like the advantages of having it in a second file, which is copied directly to the package, outweighs the benefits. Notice that with the above method, zero comments are copied over as well.
True, but usually people look at the source PKGBUILD, not at /var/lib/pacman/wherever/is/the/post-install
*** My Comments ***
This could confuse people. I already mentioned the global variables thing as a drawback, in addition to using subfunctions (see the pacman.install file). It isn't that these couldn't be solved by simply sticking with an external install file, but the confusion of educating users then comes into play. "Hey, you can put install functions in your PKGBUILD, but ONLY if they don't use variables, you don't mind missing comments, you don't try to actually use subfunctions to clean up the code...".
Maybe I'm just being a stickler here, but it seems like what we have now works quite well, at the expense of needing one extra file. It also *clearly* seperates build-time operations from install-time ones, which I can see being quite confusing to first-time PKGBUILD writers.
I agree that an external file is more straightforward, but as you said, the patch only adds a feature, it's fully backward compatible.
I wrote this mail also to say that I'm not going to develop this patch anymore, so if it doesn't apply or makes your systems explode, don't ask me to fix it because I don't have time/will to do it.
Trying to resurrect this... Dan, can this be merged for 3.2?
My comments above still apply, and I never heard great responses to them. I've marked them for you, and I'd be hard-pressed to just merge something because its been sitting around for a while. -Dan
On Mon, Feb 25, 2008 at 07:11:55AM -0600, Dan McGee wrote:
My comments above still apply, and I never heard great responses to them. I've marked them for you, and I'd be hard-pressed to just merge something because its been sitting around for a while.
I just felt on FS#3244 while looking for another bug, which made me post a comment there, which made Romashka ping this thread :) I guess I just don't like too many things being stalled. And I agree with all comments you made, so if you just closed that bug as "won't implement", at least that would be clearer. I guess it's harder to reject a patch that has been reworked several times, is documented, fully backward compatible, and implements a feature request though. But I think there are valid arguments in both cases.
participants (9)
-
Aaron Griffin
-
Alessio 'mOLOk' Bolognino
-
Alessio Bolognino
-
Dan McGee
-
eliott
-
Roman Kyrylych
-
Travis Willard
-
VMiklos
-
Xavier