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