[pacman-dev] Maybe some problems with pacman-git
I have a patch-file that I apply to makepkg in pacman-git, with some problems and enhancement. I made a new diff with the current version, and this is possible problem areas. Since I have more "feature" patches, I have tried to extract just the problems, in case someone think that the fix might be valuable. All proper UNIFIED diff patch snippets are for makepkg.sh.in BUG: When pacman recurses, and the --asroot option is given it fails to propagate --asroot to the inner pacman. The second part of the patch is that the correct options are not propagated to a recursing makepkg. I think this is primarily --noconfirm and --noprogressbar @@ -363,13 +373,15 @@ local makepkg_opts='-i -c -b' [ "$RMDEPS" = "1" ] && makepkg_opts="$makepkg_opts -r" + [ "$ASROOT" = "1" ] && makepkg_opts="$makepkg_opts -r --asroot" local ret packagedir for packagedir in $candidates; do if [ -f "$packagedir/$BUILDSCRIPT" ]; then cd "$packagedir" ret=0 - PKGDEST="$PKGDEST" makepkg $makepkg_opts || ret=$? [ $ret -eq 0 ] && continue 2 + PKGDEST="$PKGDEST" makepkg $PACMAN_OPTS $makepkg_opts || ret=$? fi done bsdtar failed to unzip one of the zip files in one of the packages, so therefore I have this patch. @@ -616,10 +628,12 @@ local file_type=$(file -bizL "$file") local cmd='' case "$file_type" in - *application/x-tar*|*application/zip*|*application/x-zip*| *application/x-cpio*) + *application/x-tar*|*application/x-cpio*) cmd="bsdtar -x -f $file" ;; *application/x-gzip*) cmd="gunzip -d -f $file" ;; + *application/zip*|*application/x-zip*) + cmd="unzip -q $file" ;; *application/x-bzip*) cmd="bunzip2 -f $file" ;; *) This one I have, since bsdtar outputs tons, and tons of error/warning messages, about something that it cannot handle attributes. Highly annoying. @@ -858,7 +876,7 @@ local pkg_file="$PKGDEST/${pkgname}-${pkgver}-${pkgrel}-${CARCH}${PKGEXT}" - if ! bsdtar -czf "$pkg_file" $comp_files $(ls); then + if ! bsdtar -czf "$pkg_file" $comp_files $(ls) 2>/dev/null ; then error "$(gettext "Failed to create package file.")" exit 1 # TODO: error code fi BUG: Pacman fails to work properly with --asroot, and when it recalls itself, on some particular packages. @@ -1425,6 +1454,7 @@ rm -rf "$pkgdir" fi mkdir -p "$pkgdir" + cd "$startdir" if [ $EUID -eq 0 ]; then # if we are root, then we don't need to recall makepkg with fakeroot @@ -1439,7 +1469,6 @@ create_package else msg "$(gettext "Entering fakeroot environment...")" - cd "$startdir" if [ "$newpkgver" != "" ]; then fakeroot -- $0 --forcever $newpkgver -F $ARGLIST || exit $? This is just a nice "feature" patch, so that you know what is going on when you use -b. @@ -969,7 +987,7 @@ install_package() { [ "$INSTALL" = "0" ] && return - msg "$(gettext "Installing package with pacman -U...")" + msg "$(gettext "Installing package ${pkgname} with pacman -U...")" if [ "$ASROOT" = "0" ]; then sudo pacman $PACMAN_OPTS -U $PKGDEST/${pkgname}-${pkgver}-${pkgrel}-${CARCH}${PKGEXT} || exit $? else
On Jan 8, 2008 4:44 AM, Karolina Lindqvist <karolina.lindqvist@kramnet.se> wrote:
@@ -363,13 +373,15 @@
local makepkg_opts='-i -c -b' [ "$RMDEPS" = "1" ] && makepkg_opts="$makepkg_opts -r" + [ "$ASROOT" = "1" ] && makepkg_opts="$makepkg_opts -r --asroot" local ret packagedir for packagedir in $candidates; do if [ -f "$packagedir/$BUILDSCRIPT" ]; then cd "$packagedir" ret=0 - PKGDEST="$PKGDEST" makepkg $makepkg_opts || ret=$? [ $ret -eq 0 ] && continue 2 + PKGDEST="$PKGDEST" makepkg $PACMAN_OPTS $makepkg_opts || ret=$? fi done
You might want to look at the second half of this patch - why did you move the call to makepkg after the continue? That'll probably break a lot.
tisdag 08 januari 2008 skrev Travis Willard:
You might want to look at the second half of this patch - why did you move the call to makepkg after the continue? That'll probably break a lot.
It's the damn unified diff. The original looks like this: - PKGDEST="$PKGDEST" makepkg makepkg_opts || ret=$? - [ $ret -eq 0 ] && continue 2 + PKGDEST="$PKGDEST" makepkg $PACMAN_OPTS $makepkg_opts || ret=$? + [ $ret -eq 0 -o $ret -eq 123 ] && continue 2 And with some editing away of the lins not belonging to this patch, the order of the lines got swapped. Karolina
On Jan 8, 2008 6:56 AM, Karolina Lindqvist <karolina.lindqvist@kramnet.se> wrote:
tisdag 08 januari 2008 skrev Travis Willard:
You might want to look at the second half of this patch - why did you move the call to makepkg after the continue? That'll probably break a lot.
It's the damn unified diff.
It might be easiest for all involved if you were to use git to generate these diffs. Dan wrote a guide just for such a purpose: http://toofishes.net/blog/git-workflow-pacman/
Karolina Lindqvist wrote:
I have a patch-file that I apply to makepkg in pacman-git, with some problems and enhancement. I made a new diff with the current version, and this is possible problem areas. Since I have more "feature" patches, I have tried to extract just the problems, in case someone think that the fix might be valuable. All proper UNIFIED diff patch snippets are for makepkg.sh.in
You might want to learn git. It does have a learning curve, but it's worth it. Read at least Dan's git guide : http://code.toofishes.net/git-guide.txt
BUG: When pacman recurses, and the --asroot option is given it fails to propagate --asroot to the inner pacman. The second part of the patch is that the correct options are not propagated to a recursing makepkg. I think this is primarily --noconfirm and --noprogressbar
@@ -363,13 +373,15 @@
local makepkg_opts='-i -c -b' [ "$RMDEPS" = "1" ]&& makepkg_opts="$makepkg_opts -r" + [ "$ASROOT" = "1" ]&& makepkg_opts="$makepkg_opts -r --asroot"
Why do you add -r in this case too?
local ret packagedir for packagedir in $candidates; do if [ -f "$packagedir/$BUILDSCRIPT" ]; then cd "$packagedir" ret=0 - PKGDEST="$PKGDEST" makepkg $makepkg_opts || ret=$? [ $ret -eq 0 ]&& continue 2 + PKGDEST="$PKGDEST" makepkg $PACMAN_OPTS $makepkg_opts || ret=$? fi done
As it has already been mentioned by Travis, this part doesn't look right. Please resubmit the corrected patch (preferably a git one ;))
bsdtar failed to unzip one of the zip files in one of the packages, so therefore I have this patch.
@@ -616,10 +628,12 @@ local file_type=$(file -bizL "$file") local cmd='' case "$file_type" in - *application/x-tar*|*application/zip*|*application/x-zip*| *application/x-cpio*) + *application/x-tar*|*application/x-cpio*) cmd="bsdtar -x -f $file" ;; *application/x-gzip*) cmd="gunzip -d -f $file" ;; + *application/zip*|*application/x-zip*) + cmd="unzip -q $file" ;; *application/x-bzip*) cmd="bunzip2 -f $file" ;; *)
Hm, which zip files exactly? This should be reported to bsdtar developers, shouldn't it?
This one I have, since bsdtar outputs tons, and tons of error/warning messages, about something that it cannot handle attributes. Highly annoying.
@@ -858,7 +876,7 @@
local pkg_file="$PKGDEST/${pkgname}-${pkgver}-${pkgrel}-${CARCH}${PKGEXT}"
- if ! bsdtar -czf "$pkg_file" $comp_files $(ls); then + if ! bsdtar -czf "$pkg_file" $comp_files $(ls) 2>/dev/null ; then error "$(gettext "Failed to create package file.")" exit 1 # TODO: error code fi
Same comment as above, on which tarballs do bsdtar produce errors or warnings?
BUG: Pacman fails to work properly with --asroot, and when it recalls itself, on some particular packages.
@@ -1425,6 +1454,7 @@ rm -rf "$pkgdir" fi mkdir -p "$pkgdir" + cd "$startdir"
if [ $EUID -eq 0 ]; then # if we are root, then we don't need to recall makepkg with fakeroot @@ -1439,7 +1469,6 @@ create_package else msg "$(gettext "Entering fakeroot environment...")" - cd "$startdir"
if [ "$newpkgver" != "" ]; then fakeroot -- $0 --forcever $newpkgver -F $ARGLIST || exit $?
Well, I am not familiar with makepkg, so I don't know what's wrong here, but your change looks safe enough. Maybe it's just an overlook since that --asroot option is probably not widely used.
This is just a nice "feature" patch, so that you know what is going on when you use -b. @@ -969,7 +987,7 @@ install_package() { [ "$INSTALL" = "0" ]&& return
- msg "$(gettext "Installing package with pacman -U...")" + msg "$(gettext "Installing package ${pkgname} with pacman -U...")" if [ "$ASROOT" = "0" ]; then sudo pacman $PACMAN_OPTS -U $PKGDEST/${pkgname}-${pkgver}-${pkgrel}-${CARCH}${PKGEXT} || exit $? else
That looks alright too, but it comes too late since pacman 3.1 is in string freeze for a while now, and will be released soon. Anyway, thanks for sharing your patches.
On Jan 8, 2008 3:44 AM, Karolina Lindqvist <karolina.lindqvist@kramnet.se> wrote:
I have a patch-file that I apply to makepkg in pacman-git, with some problems and enhancement. I made a new diff with the current version, and this is possible problem areas. Since I have more "feature" patches, I have tried to extract just the problems, in case someone think that the fix might be valuable. All proper UNIFIED diff patch snippets are for makepkg.sh.in
BUG: When pacman recurses, and the --asroot option is given it fails to propagate --asroot to the inner pacman. The second part of the patch is that the correct options are not propagated to a recursing makepkg. I think this is primarily --noconfirm and --noprogressbar
BUG: Pacman fails to work properly with --asroot, and when it recalls itself, on some particular packages.
These are the two issues I can see being important. As Xavier said, the --asroot option probably hasn't been tested much, so maybe we should look into this. If I get a chance I'll be sure to check this out before I release tonight. Of course I'd appreciate anyone else looking into it as well. -Dan
tisdag 08 januari 2008 skrev Dan McGee:
These are the two issues I can see being important. As Xavier said, the --asroot option probably hasn't been tested much, so maybe we should look into this. If I get a chance I'll be sure to check this out before I release tonight. Of course I'd appreciate anyone else looking into it as well.
One issue got left out by mistake I think. I could not get pacman to work properly building klibc, since it stripped all the symbols from /usr/lib/klibc/lib/libc.so I don't know why the old pacman did not do this, since I could not find anything different in logic. Maybe there is some other mechanism that I did not find? So just check that klibc builds without stripping /usr/lib/klibc/lib/libc.so, or install a patch similar to this. For some reason "file -biz /usr/lib/klibc/lib/libc.so" gives application/x-executable and not application/x-sharedlib, and gets its symbols stripped. That in turn will make build of klibc-* packets fail. @@ -751,7 +765,11 @@ *application/x-sharedlib*) # Libraries /usr/bin/strip --strip-debug "$file";; *application/x-executable*) # Binaries - /usr/bin/strip "$file";; + if [ "${file/.so/}" = "${file}" ]; then + /usr/bin/strip "$file"; + else + /usr/bin/strip --strip-debug "$file"; + fi;; esac done fi
On Jan 8, 2008 9:34 AM, Karolina Lindqvist <karolina.lindqvist@kramnet.se> wrote:
tisdag 08 januari 2008 skrev Dan McGee:
These are the two issues I can see being important. As Xavier said, the --asroot option probably hasn't been tested much, so maybe we should look into this. If I get a chance I'll be sure to check this out before I release tonight. Of course I'd appreciate anyone else looking into it as well.
One issue got left out by mistake I think.
I could not get pacman to work properly building klibc, since it stripped all the symbols from /usr/lib/klibc/lib/libc.so I don't know why the old pacman did not do this, since I could not find anything different in logic. Maybe there is some other mechanism that I did not find? So just check that klibc builds without stripping /usr/lib/klibc/lib/libc.so, or install a patch similar to this.
For some reason "file -biz /usr/lib/klibc/lib/libc.so" gives application/x-executable and not application/x-sharedlib, and gets its symbols stripped. That in turn will make build of klibc-* packets fail.
@@ -751,7 +765,11 @@ *application/x-sharedlib*) # Libraries /usr/bin/strip --strip-debug "$file";; *application/x-executable*) # Binaries - /usr/bin/strip "$file";; + if [ "${file/.so/}" = "${file}" ]; then + /usr/bin/strip "$file"; + else + /usr/bin/strip --strip-debug "$file"; + fi;; esac done fi
Hmm, interesting issue here. Anyone know more about this? In 3.0.6 we just used file extensions to strip so we didn't run into the above problem. [1] You can't run ldd on klibc either. Unlike /lib/libc.so.6 (glibc), this doesn't even link in ld-linux.so and linux-gate.so. This is a very corner case, and the above patch makes the code a bit sloppy looking, but I don't see many alternatives. -Dan [1] http://code.toofishes.net/gitweb.cgi?p=pacman.git;a=blob;f=scripts/makepkg;h...
tisdag 08 januari 2008 skrev Dan McGee:
You can't run ldd on klibc either. Unlike /lib/libc.so.6 (glibc), this doesn't even link in ld-linux.so and linux-gate.so. This is a very corner case, and the above patch makes the code a bit sloppy looking, but I don't see many alternatives.
If it is the only case, the symbols can just be left in klibc. I was just not sure if there was some other .so file somewhere that would give the same problem. On my two systems, it is the only case, which I figured out by for f in $(locate '*.so'); do [ "$(file -biz "$f" | awk '{print $1; }')" != "application/x-sharedlib," ] && echo "$f $(file -biz $f)"; done | grep x-executable
On Jan 9, 2008 2:26 AM, Karolina Lindqvist <karolina.lindqvist@kramnet.se> wrote:
If it is the only case, the symbols can just be left in klibc. I was just not sure if there was some other .so file somewhere that would give the same problem.
Actually, I'd prefer if the symbols were NOT in klibc. Being that this is early-userspace, we want to minimize the size and decompression overhead as the entire rest of the system is waiting for this to complete. Assuming this is a corner case, could we set things up such that options=(strip) forces the strip?
onsdag 09 januari 2008 skrev Aaron Griffin:
Actually, I'd prefer if the symbols were NOT in klibc. Being that this is early-userspace, we want to minimize the size and decompression overhead as the entire rest of the system is waiting for this to complete.
Assuming this is a corner case, could we set things up such that options=(strip) forces the strip?
The symbols are needed for building of klibc-extras, klibc-udev and I think, also klibc-module-init-tools.
On Jan 9, 2008 10:58 AM, Karolina Lindqvist <karolina.lindqvist@kramnet.se> wrote:
onsdag 09 januari 2008 skrev Aaron Griffin:
Actually, I'd prefer if the symbols were NOT in klibc. Being that this is early-userspace, we want to minimize the size and decompression overhead as the entire rest of the system is waiting for this to complete.
Assuming this is a corner case, could we set things up such that options=(strip) forces the strip?
The symbols are needed for building of klibc-extras, klibc-udev and I think, also klibc-module-init-tools.
I think you missed the point here, Aaron. It is hard to compile anything against a shared lib if it has *no symbols at all*. Actually impossible as far as I know... And options=(strip) would already force the strip, and that is the makepkg.conf default anyway. -Dan
On Jan 9, 2008 11:00 AM, Dan McGee <dpmcgee@gmail.com> wrote:
On Jan 9, 2008 10:58 AM, Karolina Lindqvist <karolina.lindqvist@kramnet.se> wrote:
onsdag 09 januari 2008 skrev Aaron Griffin:
Actually, I'd prefer if the symbols were NOT in klibc. Being that this is early-userspace, we want to minimize the size and decompression overhead as the entire rest of the system is waiting for this to complete.
Assuming this is a corner case, could we set things up such that options=(strip) forces the strip?
The symbols are needed for building of klibc-extras, klibc-udev and I think, also klibc-module-init-tools.
I think you missed the point here, Aaron. It is hard to compile anything against a shared lib if it has *no symbols at all*. Actually impossible as far as I know...
I meant debugging symbols and all that... there was a period of time when they were all being built with debug enabled.
And options=(strip) would already force the strip, and that is the makepkg.conf default anyway.
I meant this in a different way, but now realize that it sounded stupid. Even forcing the strip blows things up because it detects the type wrong, so yeah, back to square one.
On Jan 9, 2008 11:28 AM, Aaron Griffin <aaronmgriffin@gmail.com> wrote:
On Jan 9, 2008 11:00 AM, Dan McGee <dpmcgee@gmail.com> wrote:
On Jan 9, 2008 10:58 AM, Karolina Lindqvist <karolina.lindqvist@kramnet.se> wrote:
onsdag 09 januari 2008 skrev Aaron Griffin:
Actually, I'd prefer if the symbols were NOT in klibc. Being that this is early-userspace, we want to minimize the size and decompression overhead as the entire rest of the system is waiting for this to complete.
Assuming this is a corner case, could we set things up such that options=(strip) forces the strip?
The symbols are needed for building of klibc-extras, klibc-udev and I think, also klibc-module-init-tools.
I think you missed the point here, Aaron. It is hard to compile anything against a shared lib if it has *no symbols at all*. Actually impossible as far as I know...
I meant debugging symbols and all that... there was a period of time when they were all being built with debug enabled.
And options=(strip) would already force the strip, and that is the makepkg.conf default anyway.
I meant this in a different way, but now realize that it sounded stupid. Even forcing the strip blows things up because it detects the type wrong, so yeah, back to square one.
Two plans of attack here: 1. Patch makepkg with the aforementioned patch, which I find hacky as we have gotten away from anything dealing with file extensions in makepkg. 2. options=(!strip) in the klibc PKGBUILD, and do it manually as part of the build function (and stripping only debug symbols). Opinions? I don't see this as a blocker for 3.1.0 by any means as it is such a niche case and can be solved by option #2 in the short term. I'd prefer #2 in the long term too, but I don't want to be a dictator here. -Dan
On Jan 9, 2008 12:57 PM, Dan McGee <dpmcgee@gmail.com> wrote:
Two plans of attack here: 1. Patch makepkg with the aforementioned patch, which I find hacky as we have gotten away from anything dealing with file extensions in makepkg.
-1
2. options=(!strip) in the klibc PKGBUILD, and do it manually as part of the build function (and stripping only debug symbols).
+1
On Jan 9, 2008 11:57 AM, Dan McGee <dpmcgee@gmail.com> wrote:
2. options=(!strip) in the klibc PKGBUILD, and do it manually as part of the build function (and stripping only debug symbols).
I'm fine with this one, I have some patches and things incoming to klibc soon anyway, so I'd be updating the PKGBUILD then.
participants (5)
-
Aaron Griffin
-
Dan McGee
-
Karolina Lindqvist
-
Travis Willard
-
Xavier