Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 226789 - net-dialup/lrzsz: patch (lrzsz-makefile-smp.patch) changes both autotools source and result
Summary: net-dialup/lrzsz: patch (lrzsz-makefile-smp.patch) changes both autotools sou...
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: New packages (show other bugs)
Hardware: All Linux
: High normal (vote)
Assignee: Alin Năstac (RETIRED)
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: bad-autotools
  Show dependency tree
 
Reported: 2008-06-14 17:02 UTC by Diego Elio Pettenò (RETIRED)
Modified: 2009-07-10 15:13 UTC (History)
1 user (show)

See Also:
Package list:
Runtime testing required: ---


Attachments
Patch to ebuild to address various problems in the lrzsz package (lrzsz-0.12.20-r2.ebuild.patch,1.72 KB, patch)
2009-07-06 19:41 UTC, Kevin Pyle
Details | Diff
Replacement for existing Gentoo lrzsz-makefile-smp.patch (lrzsz-makefile-smp.patch,1.65 KB, patch)
2009-07-06 19:44 UTC, Kevin Pyle
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Diego Elio Pettenò (RETIRED) gentoo-dev 2008-06-14 17:02:49 UTC
The patch in summary is touching both Makefile.in and Makefile.am, probably to avoid autotools rebuild.

This treatment is usually reserved for a few selected system packages that cannot have their autotool scripts rebuilt.

This _could_ cause maintainermode-driven rebuild (see http://blog.flameeyes.eu/articles/2008/06/13/maintaner-mode ), which is something we should be avoiding as much as possible.

Please just patch Makefile.am and/or configure.in/.ac and rebuild autotools, unless you have very good reasons not to.
Comment 1 Alin Năstac (RETIRED) gentoo-dev 2008-06-22 17:33:04 UTC
Autotools source files of this package need to be fixed before you can use eautoreconf. Can you help me on that or do you want me going on "quick and dirty" path?
Comment 2 Kevin Pyle 2009-07-06 19:41:40 UTC
Created attachment 196950 [details, diff]
Patch to ebuild to address various problems in the lrzsz package

In addition to the autotools handling Diego mentioned, this package had various other problems:

- useless assignment of CFLAGS in {lib,src}/Makefile.am, triggering an automake warning
- failure to probe for locale.h in configure.in, so regenerating the configure script broke NLS support
- use of AM_GNU_GETTEXT prior to requesting the ability to use AC_COMPILE_IFELSE
- deprecated usage of AC_DEFINE/AC_DEFINE_UNQUOTED, which autoheader rejected when regenerating the autotools files
- missing config.rpath, which automake disliked
- script 'missing' was too old to understand some options used by configure
- presence of deprecated acconfig.h, which autoheader disliked
- bad install target: DESTDIR was left blank and prefix+mandir were overridden to install into the staging area
- bad install invocation: used make instead of emake, so $MAKEOPTS was ignored for install
  - files/lrsz-makefile-smp.patch did not in fact produce a parallel-safe install target, so installation failed for parallel make once the ebuild was switched to use emake install

This patch addresses all issues except the last.  For that, I have a replacement for makefile-smp.patch, which reworks the installation to use make dependencies so that it can be parallelized.  With the ebuild patch and installation patch, I can build the package successfully with MAKEOPTS=-j.
Comment 3 Kevin Pyle 2009-07-06 19:44:36 UTC
Created attachment 196955 [details, diff]
Replacement for existing Gentoo lrzsz-makefile-smp.patch

This patch is meant to replace the existing lrzsz-makefile-smp.patch.  This patch allows parallel install, where the original patch did not.  Caveat: since the ebuild does not use the program name transformation feature of Autotools, I dropped support for that to simplify the patch.
Comment 4 Alin Năstac (RETIRED) gentoo-dev 2009-07-10 00:09:07 UTC
Fixed in lrzsz-0.12.20-r3.

I've used a patch (autotools.patch to be more exact) to apply all your changes. I had to do some changes here and there, but all in all, great job with those autotools patches!

As for the parallelized make install, I'm sure users all over the world will thank you for the speed gain. ;)
Comment 5 Kevin Pyle 2009-07-10 02:43:44 UTC
Parallel install is not that big an issue here, but I try to fix it wherever I find it, so that automated checks for serial installation can be used to find cases where it would matter, rather than producing noise reporting issues that are easily fixed.

If I may ask, why did you switch from using hard links to symbolic links?  Portage seems to preserve the use of hard links when it installs to the live filesystem, and the upstream Makefile had done hard links in its install target.
Comment 6 Alin Năstac (RETIRED) gentoo-dev 2009-07-10 06:45:24 UTC
(In reply to comment #5)
> Parallel install is not that big an issue here, but I try to fix it wherever I
> find it, so that automated checks for serial installation can be used to find
> cases where it would matter, rather than producing noise reporting issues that
> are easily fixed.

But -r2 doesn't generate any QA notices. 

> If I may ask, why did you switch from using hard links to symbolic links? 
> Portage seems to preserve the use of hard links when it installs to the live
> filesystem, and the upstream Makefile had done hard links in its install
> target.

I guess I like the symbolic links better because it is easier to spot them. Besides, src_install creates some symbolic links after make install, so I thought, why mixing?
Comment 7 Kevin Pyle 2009-07-10 15:11:10 UTC
(In reply to comment #6)
> But -r2 doesn't generate any QA notices. 

For you. ;)  Among other things, I have a Portage bashrc snippet that replaces make with a bash function that prints a QA warning, then calls emake.  It is a handy way of quickly locating ebuilds that use make, and thereby ignore $MAKEOPTS, without searching the entire tree, which could flag hundreds of packages I do not use and have no time to fix.

If you want to incorporate it, add the below function at global scope to /etc/portage/bashrc.  Beware that since it chains to emake, it will honor $MAKEOPTS where they were ignored before, which could cause a parallel build failure if $MAKEOPTS specifies parallel build and the package was using 'make' rather than 'emake -j1' to express the need for serial operation.

make() {
	eqawarn "/etc/portage/bashrc QA notice: 'make' called by ${FUNCNAME[1]}: ${CATEGORY}/${PF}"
	emake "$@"
}

> I guess I like the symbolic links better because it is easier to spot them.
> Besides, src_install creates some symbolic links after make install, so I
> thought, why mixing?

Fair enough.  Hard links might be very slightly more efficient since it avoids a symlink read and follow operation, but I doubt anyone will ever detect _that_ difference outside some fairly targeted benchmark applications.
Comment 8 Diego Elio Pettenò (RETIRED) gentoo-dev 2009-07-10 15:13:45 UTC
Uh thanks Kevin, I think I'll be adding that to the tinderbox's bashrc now :D