Re: [pacman-dev] patch submission help
On Tue, Mar 17, 2009 at 11:17:20AM +1000, Allan McRae wrote:
Note that with the changes that are now in the pacmans git repo, there is support for build() and package() functions in PKGBUILDs. When the package() function is present, it is run under fakeroot but the build() function is not. Otherwise it is backwards compatible.
This can be dangerous, unless the pacman devs personally undertake the migration of all extant PKGBUILDs. Since I just realized the problem yesterday, I have not had time to think about the best fix, but I think the fix should be rather trivial at least in reference to my patch. I'd really like to see your branch and I could send you my patch (onlist/offlist your choice) as a matter of reference if you would like my input.
I like the idea of an additional check() function. However, I could implement it slightly differently - instead of a makepkg.conf flag to run the check() function or not, I think it may be better to add a --check flag to makepkg so people specify when to run it.
From a technical standpoint, I prefer the --check arg better than putting an option override in a PKGBUILD, but then I also like the option override as a "hint" to the individual who is building packageX that the testsuite _should_ be run. I'll keep using my modified pacman-3.2.2 until I finish the current rebuild, but after that I'd be happy to test the changes. I have no interest in maintaining my own set of patches. :)
As a side note, I've modified dozens of PKGBUILDs to implement this change, and I'm rather clueless as to why all the || return 1 statements exist since set -e is in use. Are they cruft left behind from an earlier version of makepkg that didn't bail on the first error? Also, my patch actually just runs run_build() with an argument (the arg being the name of the PKGBUILD function to run) so there was no code duplication. In run_build() I put in a simple: cd "${srcdir}/${pkgname}-${pkgver}" || cd "${srcdir}" as it seemed silly to have a cd command in every single PKGBUILD when a good number of tarballs follow the ${pkgname}-${pkgver} naming scheme. I don't know if you have added that feature or not. Especially with more than one function in PKGBUILD, cd would have to be duplicated (and still will for non-standard source tarballs). A function for determining that name might be useful, though, and would help in all cases except where the build dir != source dir (like glibc). Anyway, just food for thought! -- Jeff My other computer is an abacus.
Jeff wrote:
On Tue, Mar 17, 2009 at 11:17:20AM +1000, Allan McRae wrote:
Note that with the changes that are now in the pacmans git repo, there is support for build() and package() functions in PKGBUILDs. When the package() function is present, it is run under fakeroot but the build() function is not. Otherwise it is backwards compatible.
This can be dangerous, unless the pacman devs personally undertake the migration of all extant PKGBUILDs. Since I just realized the problem yesterday, I have not had time to think about the best fix, but I think the fix should be rather trivial at least in reference to my patch. I'd really like to see your branch and I could send you my patch (onlist/offlist your choice) as a matter of reference if you would like my input.
What exactly is dangerous about this? There are two options 1) build() and package() functions - here only package() is run in fakeroot 2) build() function only - build() is run in fakeroot. All my changes are now on the pacman master branch so you can just check that out. See here for further info: http://www.archlinux.org/pacman/#_development
I like the idea of an additional check() function. However, I could implement it slightly differently - instead of a makepkg.conf flag to run the check() function or not, I think it may be better to add a --check flag to makepkg so people specify when to run it.
From a technical standpoint, I prefer the --check arg better than putting an option override in a PKGBUILD, but then I also like the option override as a "hint" to the individual who is building packageX that the testsuite _should_ be run. I'll keep using my modified pacman-3.2.2 until I finish the current rebuild, but after that I'd be happy to test the changes. I have no interest in maintaining my own set of patches. :)
The check() function should serve as a major hint that it should be run. Perhaps, if it is present and not invoked using --check then a warning could be printed.
As a side note, I've modified dozens of PKGBUILDs to implement this change, and I'm rather clueless as to why all the || return 1 statements exist since set -e is in use. Are they cruft left behind from an earlier version of makepkg that didn't bail on the first error? Also, my patch actually just runs run_build() with an argument (the arg being the name of the PKGBUILD function to run) so there was no code duplication. In run_build() I put in a simple:
cd "${srcdir}/${pkgname}-${pkgver}" || cd "${srcdir}"
as it seemed silly to have a cd command in every single PKGBUILD when a good number of tarballs follow the ${pkgname}-${pkgver} naming scheme. I don't know if you have added that feature or not. Especially with more than one function in PKGBUILD, cd would have to be duplicated (and still will for non-standard source tarballs). A function for determining that name might be useful, though, and would help in all cases except where the build dir != source dir (like glibc). Anyway, just food for thought!
There is currently nothing like that implemented. I have thought about saving the current directory from build() and setting to that by default when you move into the package() function. That would be good as I see a PKGBUILD as just the collection of commands you would type manually and you wouldn't be doing all these extra cd. I am not sure about the auto cd you have proposed. Allan
participants (2)
-
Allan McRae
-
Jeff