[pacman-dev] patch submission help

[Allan McRae]

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