FS#15946 - [devtools] makechrootpkg does not always copy all necessary files to /build directory

Attached to Project: Arch Linux
Opened by Krzysztof Raczkowski (raku) - Saturday, 15 August 2009, 21:05 GMT
Last edited by Allan McRae (Allan) - Saturday, 05 September 2009, 02:34 GMT
Task Type Bug Report
Category Packages: Extra
Status Closed
Assigned To Aaron Griffin (phrakture)
Allan McRae (Allan)
Architecture All
Severity Medium
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 1
Private No

Details

Description:
I work with makechrootpkg while compiling AUR packages. I noticed, that it doesn't handle copying of additional files needed for building a package (foo.install, etc). They are copied to /srcdir, but sometimes, they (also) need to be placed directly in /build directory.
The problem occurs when:
-) PKGBUILD has a variable install=${package}.install instead of install=foo.install
-) there are obsolete paths to files in build() function, for example: cp ${startdir}/foo.desktop (copy file directly from main PKGBUILD directory) instead of cp ${srcdir}/foo.desktop (copy from symlink pointing to file from main PKGBUILD directory).

Here's an example when makechrootpkg fails (splashy package from AUR):
$ ls
PKGBUILD splash splashy-0.3.13-2-x86_64.pkg.tar.gz splashy.initcpio_hook splashy.install
PKGBUILD~ splash.conf splashy-functions splashy.initcpio_install

$ grep install= PKGBUILD
install=${pkgname}.install

$ sudo makechrootpkg -c -r /var/cache/chroots/arch64/ -- --noconfirm
building union chroot
moving build files to chroot
Setting PKGDEST in makepkg.conf
Setting SRCDEST in makepkg.conf
allowing 'nobody' sudo rights in the chroot
mounting sysfs : /sys
mounting procfs : /proc
binding device nodes : /dev
binding pacman cache : /var/cache/pacman
copying mtab : /etc/mtab
copying resolv.conf : /etc/resolv.conf
starting chroot (/chrootbuild)
==> ERROR: Install scriptlet (splashy.install) does not exist.
cleaning up mounts
Build failed, check /var/cache/chroots/arch64/rw/build
cleaning up unioned mounts

I know, that should be fixed in PKGBUILD, but there are so many broken PKGBUILDs, that I think it would be better to add one additional line to makechrootpkg script (included in attachment) which fix this issue.


Additional info:
* package version(s):
- devtools 0.6.4-1
- devtools-git 20090815-1
This task depends upon

Closed by  Allan McRae (Allan)
Saturday, 05 September 2009, 02:34 GMT
Reason for closing:  Fixed
Additional comments about closing:  commit f9aa28f8
Comment by Ionut Biru (wonder) - Saturday, 15 August 2009, 22:13 GMT
i know about this issue. i've talk with eric about it and the conclusion was to try fix the PKGBUILDs.
Comment by Krzysztof Raczkowski (raku) - Saturday, 15 August 2009, 22:22 GMT
but it could be the never ending story :-(
Maybe a workaround in makechrootpkg would be a better solution?
instead of copying files to /build it would be better to symlink them from /srcdir:
ln -s "/srcdest/$basef" "$uniondir/build/$basef"
Comment by Eric Belanger (Snowman) - Saturday, 15 August 2009, 22:46 GMT
I'll try to see if i can't come up with a fix.

BTW, things like cp ${startdir}/foo.desktop are totally incorrect (namcap even throughs a warning or error). In this case, the PKGBUILD must be fixed.
Comment by Jan de Groot (JGC) - Saturday, 15 August 2009, 23:26 GMT
Copying straight out of $startdir is wrong IMO, that also bypasses the need for adding the file to the sources array. PKGBUILDs doing that should be fixed, no matter how much of them do it that way.
Comment by Eric Belanger (Snowman) - Sunday, 16 August 2009, 00:24 GMT
I checked the makechrootpkg in git for the install file issue. The problem is that to find the install file names it uses
install_files=$(grep "install=" PKGBUILD)
For unknow reasons, after doing that there is no variable substitutions possible, i.e. ${pkgname} is treated litterally. Even if we fix that, there is the problem that splitted packages use an array for the pkgname, so what value of ${pkgname} it to be used? I don't know how to do this.

An easy fix would be to simply copy all the *.install files in the build root: cp *.install "$uniondir/build/"
This will break for install scriptlets that don't use the .install extension but, in my experience, everyone uses this extension. So the number of users affected will be minimal if not zero.

Comment by Krzysztof Raczkowski (raku) - Sunday, 16 August 2009, 07:42 GMT
> I checked the makechrootpkg in git for the install file issue. The problem is that to find the install file names it uses
> install_files=$(grep "install=" PKGBUILD)
> For unknow reasons, after doing that there is no variable substitutions possible, i.e. ${pkgname} is treated litterally.

Try this code:
source PKGBUILD
install_files=$(grep "install=" PKGBUILD)
install_files=$(eval echo $install_files)


> Even if we fix that, there is the problem that splitted packages use an array for the pkgname, so what value of ${pkgname}
> it to be used? I don't know how to do this.
My code should substitute all variables with their values, so - no matter how you prefix .install file, if it is a variable used in PKGBUILD, it would work.
Comment by Eric Belanger (Snowman) - Monday, 17 August 2009, 04:49 GMT
Thanks for the eval tip. I haven't thought about it. Anyway, your fix wasn't working with splitted packages using install=${pkgname}.install but I figured a way to do it. I'm attaching the patch.
Comment by Aaron Griffin (phrakture) - Tuesday, 18 August 2009, 22:47 GMT
Hmm the "eval package_$pkg" part worries me. It's going to actually run that function to discover the install files.

You know what would solve this? Simply putting the install files in the sources array. This would give us file checksums as well...
Comment by Eric Belanger (Snowman) - Tuesday, 18 August 2009, 23:48 GMT
If eval does this then, yes it might be a problem. I only tested with a dummy PKGBUILD. It's the only way I could find out to get the .install files names in all cases.

If we want to start putting the install file in the source array, we'll need to fix makepkg. It currently treat .install files differently than source so putting the .install in the source array breaks the sourceball options.
Comment by Allan McRae (Allan) - Saturday, 05 September 2009, 02:34 GMT

Loading...