[Fedora-packaging] Re: Modifying upstream tarballs

[Patrice Dumas]

On Wed, Jun 06, 2007 at 09:35:46PM +0200, Axel Thimm wrote:
> 
> A one-liner in configure.ac can generate tons of shell script code,
> what you would be reviewing is functionality of the autotools, and in
> fact by submitting the package this way, this is what you implicitely
> do.

If it is the case, then, right it may be better not to provide a patch.
But I personally find that not that hard to review the changes in
configure, especially when one get used to.

> Reviewing the same set of patches that you *didn't* generate yourself,
> but got as an external patch to configure is far less reliables and in
> fact a must.

Clearly yes.

> So you're far better off to use autotools than to trust or review Joe
> Random's use of it, as this is what an external patch to generated
> files is.

Not necessarily, if the patch looks sane and it is known to work.
 
> > In some cases the patcheset is too big and complicated such that it
> > may become right to call the autotools, but for many changes a diff
> > to be reviewed for autotool generated files is better.
> 
> If the diff is really just replacing foo with foo2 or similar then I
> agree, but if it involves modifed output due to changes in the
> autotools workflow, then by all means no.

Sorry, but I don't understand what you are meaning...

> > The fact that something exists doesn't mean that it should be abused. 
> 
> No, you trimmed the sentence I replied to. There was a claim that
> maintainers are not supposed to use autotools while autotools has an
> explicit mechanism for maintainers which is also called that way. I
> didn't say that --enable-maintainers is supposed to be used.
> 
> And note, that AM_MAINTAINER_MODE defaults to --enable-maintainers if
> not used!!! Which means that 99% of all projects already "abuse" this
> if they want to or not.

I disagree. I don't think this feature is for released packages, but it
is to be used between releases only. Of course I don't want to force
anybody ;-).

> > There should be both in the diff: a diff to the Makefile.am file, for 
> > example, and also the change to the Makefile.in. That way you have both.
> 
> And what does the latter buy us? A patch to a generated file is very
> difficult to comprehend and verify unless it is a really trivial patch
> to the master source. You will have to verify the patch to the
> generated file by having a recipe on how to create it, which autotools
> called with what options, so why not embed that recipe into the
> specfile's scriptlet and only worry about the master file changes?

To verify the patch it is better to have a recipe to recreate it, sure,
(it could be in comments in the spec file for example) but to review it, 
the diff of the generated file may be enough.

> > > And again: If the derived source code changes are not reproducible
> > > with Fedora tools, then the package is neither revieable, nor
> > > maintainable in Fedora space at all.
> > 
> > Not necessarily, if the patch to the autotool generated files are clear
> > enough.
> 
> I agree. In simple replacements this will work. The problem is when to
> draw the line, as everyone will define "simple" differently.

Sure. All that should be left to the submitter and reviewer of course.
In my experience, it was useful for a2ps not to rerun the autotools,
for automake1* the patches are easy to review, but for grads (I
maintain) and coreutils, the changes are so big that it isn't practical
to give a patch.

> > I don't think rerunning autotools should be avoided at all costs, but 
> > I have seen many reviews where it is easier to review the changes by
> > providing patches for the primary files and for the generated files,
> > to understand what changed also in the generated files.
> 
> Why? What made these reviews so problematic? And why can't the
> reviewer just look at what is changing in the generated files on his
> system? How can the reviewer trust the submitter that the latter used
> autotools properly and didn't manually fix some things in the
> generated patches?

By reviewing the patches. 

> You will end up that the reviewer needs to perform the same steps that
> could had been automated in the specfile, and noone would have to look
> at how the generated configure/Makefile.in looks like if he reviews
> the chnaged to the master files and the build is conducted giving the
> expected results.

Indeed a reviewer can do the patch himself and verify it. For the
submitter adding the generated files patches and fixing the timestamps
is more work, but it allows for some reviewers to skip doing it, and
more importantly it freezes the patch to the autotool version that was
used for the review.

> You don't review changes to assembly code that wer induced by changes
> to the C code either.

Of course, but assembly diff is much harder to read than autotool
generated files diff. But maybe there was also some humor, here...

--
Pat