Bug 1320210

Without looking at the patch, I've got some changes in a local branch which I've been meaning to discuss with the other (more active) git maintainers.  I also think it's very useful to keep the spec file compatible with older EL releases, to the extent possible.  (I suspect that el5 will eventually become a pain to support, just as el4 did with age).

I'll take a look and see if there are any fixes you've made which I missed.  I built for el6 only most recently, so I could have missed something for el5.

Created attachment 1139161 [details]
Conditionalize bash-completion pkg-config usage for EL <= 6

Here's how I'd do the bash completion bits.  It leaves less conditional statements in the body of the spec file.  It also ensures ownership of the bash completion dir (which might eventually need to change in as future fedora releases have this in the filesystem package, IIRC).

Created attachment 1139162 [details]
Improve el5 and el6 support

This adds Konrad's two additional fixes for better el5/el6 support.

Your patch for the bash-completion is much better. Thanks for the quick response.

Created attachment 1140612 [details]
Conditionalize bash-completion pkg-config usage for EL <= 6

Thanks for contribution. It makes sense. However there is little problem with using *pkg-config* to get %bashcompdir. It makes troubles when I want to build package for rawhide (mockbuild). pkg-config works only with installed packages and bash-completion isn't installed from the start of processing. Probably this causes troubles during compilation of the git.

So I rather replace it by path directly: /usr/share/bash-completion/completions

Todd, do You agree with this change or do You prefer different solution? Otherwise patches can be applied. Feel free to apply it in rawhide if you agree.

Petr

Hi Petr.  The warning/error output from pkg-config is only when mock (re)builds the src rpm.  During the "real" rpm build, it works as it should since we have "BuildRequires: bash-completion" (conditionally, based on the %{bashcomp_pkgconfig} macro).

This is similar to the errors seen when calling python to determine the site lib in some macros.  It's harmless, but it is a bit noisy.  I can easily fix it with '2>/dev/null' in the pkg-config line (and have done so locally).

Here's my mockbuild output from today:

$ rpm -qplv results_git/2.7.4/1.fc25/git-core-2.7.4-1.fc25.x86_64.rpm | grep bash-completion
drwxr-xr-x    2 root    root        0 Mar 26 12:42 /usr/share/bash-completion
drwxr-xr-x    2 root    root        0 Mar 26 12:42 /usr/share/bash-completion/completions
-rw-r--r--    1 root    root    57390 Mar 26 12:42 /usr/share/bash-completion/completions/git
lrwxrwxrwx    1 root    root        3 Mar 26 12:42 /usr/share/bash-completion/completions/gitk -> git

Let me know if you have different results or any other concerns with the suggested patches.  If not, I'll get them applied to the master branch.

Ok now I am confused. I have tried another build (twice) with original patch and it works now.

Earlier, for original patch and even with added line 'BuildRequires: bash-completion' I couldn't build package. Or more precisely - I could without Konrad's patch. When I apply both patches, mockbuild fails (only mockbuild, local build was ok too if I remember well).It failed during %build phase - warning messages above weren't main problem. But I have not saved previous build.log and can't reproduce it again.

So You can apply even original patch.

I pushed these to master the other day and they've now been built as part of the 2.8.0 update.  Thanks Konrad and Petr. :)