Re: [Gnushogi-devel] [PATCH] my configure.ac patch

[Yann Dirson]

Hi,

On Tue, Jul 15, 2014 at 02:44:44PM +0800, Chen Chen wrote:
> Here's my patch. Hope I'm using "git send-mail" correctly.

Not bad for a first ;)

It's nevertheless a good idea to split commits when it includes
changes that are described differently.

The automake 1.14 note states that AM_PROG_CC_C_O is obsolescent and
included in AM_PROG_CC starting with 1.14.  I'm adjusting your commit
message to reflect that, it's straightforward enough and described
shortly enough not to be separated from the AC_CONFIG_HEADERS change.

However, the makeinfo part is sufficiently different that you
(correctly) described it separately, a separate commit would have been
good, and in fact that part raises further questions:

* the build truly fails when makeinfo is not installed, which shows
  your concern is legitimate

* OTOH, I'm not sure it is reasonable to have the build fail when
  makeinfo is missing, I'd think a warning ought to be enough,
  especially as the .info file is included in the dist tarball, and is
  more recent than the .texi, and as such its rebuild should not be
  triggered by default.

I'll have to dive into this, and will just apply the first part of
your patch for now.


> Following your advices (Yann, H.G. thank you), I will package the srpm
> as two parts: the original git snapshot and the patch.
> 
> The result is not so smooth, since it require "autoreconf -i" in
> "%prep" stage of rpmbuild, thus introducing dependency requirements on
> autoconf, m4, perl etc., but I'll try it.

Once the patch is in master, this won't be necessary anymore.
Hopefully the makeinfo part is not critical for your package, and
you'll be able to use the new master.


> Another problem is, the package cannot properly "make distcheck".
> It seems the two ".inc" files are generated by "pat2inc" during build
> time but didn't get proper deletion in "make clean" stage.

Good catch, I apparently missed some extra declaration.  Also noted in
README.maint not to forget "make distcheck".


> In addition, I don't know what these two files are doing. In a rough
> skim reading, I didn't find any source files utilizing them.

"git grep -F .inc" shows they are included by pattern.c.


> After some fine tweaks I'll update the srpm package on bugzilla and in
> the list.
> 
> Response to Yann:
> 
> > Well, you can still run "make install prefix=/whatever/usr", although it
> > is not as modern :)
> 
> Yes I can, but then I need a handmade "make install prefix=%{buildroot}"
> instead of macro "%make_install", and it will (at least on my PC) fire
> a "check-buildroot failure" in rpmbuild finalization because the generated
> spec file will contain absolute path.

Well, I'm not familiar enough with rpmbuild details to known better :)
It's not that hard to add DESTDIR support to the old Makefiles in 1.4,
I'll surely find the time for that.

Thanks for your contribution!
-- 
Yann