I understand that there are circumstances where the upstream tarballs need to be modified to removed content which is patented or cannot be redistributed for some reason. However, should that be the extent of the permitted modifications?
Take, for example, this comment from a current review ticket:
#Original from http://membres.lycos.fr/agisite/prof.zip includes #copyrighted executables. Generated new source by unzipping, removing #DOS-related content, running dos2unix on the text file, and changing #all filenames to lowercase for agistudio compatibility.
Now, everything except the removal of the executables could be done at %prep time in the spec, and that's how we'd insist that things be done in the normal case where the upstream tarball (or zipfile, in this case) needs no modification.
- J<
On 04 Jun 2007 18:04:37 -0500, Jason L Tibbitts III tibbs@math.uh.edu wrote:
I understand that there are circumstances where the upstream tarballs need to be modified to removed content which is patented or cannot be redistributed for some reason. However, should that be the extent of the permitted modifications?
Take, for example, this comment from a current review ticket:
#Original from http://membres.lycos.fr/agisite/prof.zip includes #copyrighted executables. Generated new source by unzipping, removing #DOS-related content, running dos2unix on the text file, and changing #all filenames to lowercase for agistudio compatibility.
Now, everything except the removal of the executables could be done at %prep time in the spec, and that's how we'd insist that things be done in the normal case where the upstream tarball (or zipfile, in this case) needs no modification.
I'm not sure I see what your point is - the website referred to in the comment is uninformative in the extreme. What's the BZ # for the review ticket?
"JU" == Jonathan Underwood jonathan.underwood@gmail.com writes:
JU> I'm not sure I see what your point is
To ask the simple question stated in the first paragraph of my message:
In the case that content must be removed from the upstream tarball, should the modifications be limited to the removal of the content or are other changes also allowed?
JU> What's the BZ # for the review ticket?
Well, it's immaterial to the question, but if it interests you, it's https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=241761
- J<
On Monday 04 June 2007 19:04:37 Jason L Tibbitts III wrote:
I understand that there are circumstances where the upstream tarballs need to be modified to removed content which is patented or cannot be redistributed for some reason. However, should that be the extent of the permitted modifications?
I would prefer it to just be the absolutely necessary changes, doing the rest later in the package. In the case that the inappropriate content goes away upstream, then we can immediately switch to an unmodified source.
On Monday 04 June 2007 19:56:59 Jesse Keating wrote:
On Monday 04 June 2007 19:04:37 Jason L Tibbitts III wrote:
I understand that there are circumstances where the upstream tarballs need to be modified to removed content which is patented or cannot be redistributed for some reason. However, should that be the extent of the permitted modifications?
I would prefer it to just be the absolutely necessary changes, doing the rest later in the package. In the case that the inappropriate content goes away upstream, then we can immediately switch to an unmodified source.
Oh, and it makes it far easier to verify the upstream vs what is in the package when only the necessary changes are made. If more and more changes are made it makes it more and more difficult to verify.
On Mon, 2007-06-04 at 18:04 -0500, Jason L Tibbitts III wrote:
I understand that there are circumstances where the upstream tarballs need to be modified to removed content which is patented or cannot be redistributed for some reason. However, should that be the extent of the permitted modifications?
The only time we should be modifying tarballs is when the tarball would cause us to ship something in the SRPM that Fedora:
- doesn't have permission to distribute - doesn't permit because of its license - would be violating known patents to ship
In 99% of the other cases, leave the tarball alone and edit the content in the spec.
~spot
On Di Juni 5 2007, Tom "spot" Callaway wrote:
The only time we should be modifying tarballs is when the tarball would cause us to ship something in the SRPM that Fedora:
- doesn't have permission to distribute
- doesn't permit because of its license
- would be violating known patents to ship
In 99% of the other cases, leave the tarball alone and edit the content in the spec.
There are two packages I am working on, that contain binaries. Is it o.k. to keep them in the tarball / src.rpm?
Regards, Till
On Tue, 2007-06-05 at 16:11 +0200, Till Maas wrote:
On Di Juni 5 2007, Tom "spot" Callaway wrote:
The only time we should be modifying tarballs is when the tarball would cause us to ship something in the SRPM that Fedora:
- doesn't have permission to distribute
- doesn't permit because of its license
- would be violating known patents to ship
In 99% of the other cases, leave the tarball alone and edit the content in the spec.
There are two packages I am working on, that contain binaries. Is it o.k. to keep them in the tarball / src.rpm?
If the binaries are under an OK for Fedora license, and it is clear that there is explicit permission to distribute (usually from the license), then, yes, it is fine, but you can't put them in the Binary RPM.
~spot
On Tuesday 05 June 2007, Jason L Tibbitts III wrote:
I understand that there are circumstances where the upstream tarballs need to be modified to removed content which is patented or cannot be redistributed for some reason. However, should that be the extent of the permitted modifications?
Take, for example, this comment from a current review ticket:
[...]
There are cases where further modifications are useful, eg. in order to eliminate build dependencies.
But anyway, no matter what the changes to the tarballs/zipfiles/whatever are, just like in the case of tar'ing up cvs/svn/etc snapshots, all of them should be done with a script, and the script included in the source rpm plus a comment in the specfile like "SourceX generated from vanilla upstream tarball with SourceY". That helps everyone involved with the package.
"VS" == Ville Skyttä ville.skytta@iki.fi writes:
VS> There are cases where further modifications are useful, eg. in VS> order to eliminate build dependencies.
I'm having trouble figuring out a case where you'd need to do this. The only thing I can come up with is that the upstream source is archived in a format other than zip or tar and you want to avoid a build-time dependency on the archiver.
- J<
On Tuesday 05 June 2007, Jason L Tibbitts III wrote:
"VS" == Ville Skyttä ville.skytta@iki.fi writes:
VS> There are cases where further modifications are useful, eg. in VS> order to eliminate build dependencies.
I'm having trouble figuring out a case where you'd need to do this. The only thing I can come up with is that the upstream source is archived in a format other than zip or tar and you want to avoid a build-time dependency on the archiver.
Let's say for example one modifies the upstream tarball to prune things not allowed in Fedora, and while doing so, touch some files which require re-running autotools.
It's sometimes pretty hard to convince autotools not to run during the build after patching/touching something, and the upstream thing might need a version of autotools that is not available in all target distro versions, so that's not an easy way out either.
I think running autotools locally before re-rolling the modified tarball instead of doing the absolute minimum changes would be ok in this case, as long as things are scripted/documented.
Ville Skyttä wrote:
I think running autotools locally before re-rolling the modified tarball instead of doing the absolute minimum changes would be ok in this case, as long as things are scripted/documented.
I'm uncomfortable with that, and prefer the consistency/reproducibility of running autotools at buildtime, but that's just me.
-- Rex
On Tue, 2007-06-05 at 12:40 -0500, Rex Dieter wrote:
Ville Skyttä wrote:
I think running autotools locally before re-rolling the modified tarball instead of doing the absolute minimum changes would be ok in this case, as long as things are scripted/documented.
I'm uncomfortable with that, and prefer the consistency/reproducibility of running autotools at buildtime, but that's just me.
This approach is the guaranteed way to ruin, because
1. The autotools are not supposed to be run at built time.
2. Many older package configurations do not work with recent autotools and break in often subtile ways if you run newer autotools on them.
3. There is nothing reliable in running the autotools at buildtime.
Finally, it's not hard not add magic to configurations in such a way they don't re-run the autotools.
Ralf
On Tue, Jun 05, 2007 at 09:04:01PM +0200, Ralf Corsepius wrote:
On Tue, 2007-06-05 at 12:40 -0500, Rex Dieter wrote:
Ville Skyttä wrote:
I think running autotools locally before re-rolling the modified tarball instead of doing the absolute minimum changes would be ok in this case, as long as things are scripted/documented.
I've never run into a package whose autotools was not supported in some version in Fedora, and if that kind of package does exist, then it is even harder to redo the steps, so we will lose reproducablity of sources.
I'm uncomfortable with that, and prefer the consistency/reproducibility of running autotools at buildtime, but that's just me.
This approach is the guaranteed way to ruin, because
- The autotools are not supposed to be run at built time.
Unless configure.ac/Makefile.ams are patched.
- Many older package configurations do not work with recent autotools
and break in often subtile ways if you run newer autotools on them.
That's why we have tons of auto*<version> packages to cover all cases.
- There is nothing reliable in running the autotools at buildtime.
Looks like a repetition of point 1. :)
Autotools have been known to provide deterministic results just like any other software. ;)
Finally, it's not hard not add magic to configurations in such a way they don't re-run the autotools.
Can you elaborate what this means when configure.ac/Makefile.am have been modified? You must either redo the autotooling or ship a second patch that is applied after a (u)sleep to the first patch. And reviewing a patch to configure/Makefile.in to verify it is indeed the derived patch from configure.ac/Makefile.am is no fun.
On Wed, 2007-06-06 at 07:42 +0200, Axel Thimm wrote:
On Tue, Jun 05, 2007 at 09:04:01PM +0200, Ralf Corsepius wrote:
On Tue, 2007-06-05 at 12:40 -0500, Rex Dieter wrote:
Ville Skyttä wrote:
I think running autotools locally before re-rolling the modified tarball instead of doing the absolute minimum changes would be ok in this case, as long as things are scripted/documented.
I've never run into a package whose autotools was not supported in some version in Fedora, and if that kind of package does exist, then it is even harder to redo the steps, so we will lose reproducablity of sources.
I'm uncomfortable with that, and prefer the consistency/reproducibility of running autotools at buildtime, but that's just me.
This approach is the guaranteed way to ruin, because
- The autotools are not supposed to be run at built time.
Unless configure.ac/Makefile.ams are patched.
Then patch the generated files, too.
- Many older package configurations do not work with recent autotools
and break in often subtile ways if you run newer autotools on them.
That's why we have tons of auto*<version> packages to cover all cases.
Well, we have some RH-patched versions around, but we don't necessarily have the versions around the original authors used. The might have been using differently patched versions originating from other vendors or even custom versions.
So, even using the RH-patched versions resembling to the original versions isn't guaranteed to work.
- There is nothing reliable in running the autotools at buildtime.
Looks like a repetition of point 1. :)
1. was poorly phrased ;) It should have been "the autotools are not designed to be run at buildtime".
Autotools have been known to provide deterministic results just like any other software. ;)
If people were using vanilla versions and if vendors would should vanilla versions, yes.
Finally, it's not hard not add magic to configurations in such a way they don't re-run the autotools.
Can you elaborate what this means when configure.ac/Makefile.am have been modified?
Also patch the generated files and adjust timestamps on generated and source files. What to do exactly depends on the version of auto*tools being used and on a configuration's details.
You must either redo the autotooling or ship a second patch that is applied after a (u)sleep to the first patch.
Nah, adjust the time stamps after applying the patches:
E.g. in one real-world case I maintain touch -r configure.in config.h.in does the job.
What to do exactly depends on a package's details (check the a package's toplevel Makefile.in).
The trick is to adjust timestamps between sources (configure.{in,ac}, Makefile.am, and generated files (configure, aclocal.m4, Makefile.in, etc.), using the source files as time-stamp reference files.
Ralf
On Wed, Jun 06, 2007 at 08:41:30AM +0200, Ralf Corsepius wrote:
On Wed, 2007-06-06 at 07:42 +0200, Axel Thimm wrote:
On Tue, Jun 05, 2007 at 09:04:01PM +0200, Ralf Corsepius wrote:
On Tue, 2007-06-05 at 12:40 -0500, Rex Dieter wrote:
Ville Skyttä wrote:
I think running autotools locally before re-rolling the modified tarball instead of doing the absolute minimum changes would be ok in this case, as long as things are scripted/documented.
I've never run into a package whose autotools was not supported in some version in Fedora, and if that kind of package does exist, then it is even harder to redo the steps, so we will lose reproducablity of sources.
I'm uncomfortable with that, and prefer the consistency/reproducibility of running autotools at buildtime, but that's just me.
This approach is the guaranteed way to ruin, because
- The autotools are not supposed to be run at built time.
Unless configure.ac/Makefile.ams are patched.
Then patch the generated files, too.
- Many older package configurations do not work with recent autotools
and break in often subtile ways if you run newer autotools on them.
That's why we have tons of auto*<version> packages to cover all cases.
Well, we have some RH-patched versions around, but we don't necessarily have the versions around the original authors used. The might have been using differently patched versions originating from other vendors or even custom versions.
So, even using the RH-patched versions resembling to the original versions isn't guaranteed to work.
In that case this means we would never be able to verify the pathces at all, so an argument to not even let the package pass.
- There is nothing reliable in running the autotools at buildtime.
Looks like a repetition of point 1. :)
- was poorly phrased ;) It should have been "the autotools are not
designed to be run at buildtime".
Why? I see nothing in the design that implies that. In fact autotools promote autorebuilds when a user modifies the sources of the generated files.
Autotools have been known to provide deterministic results just like any other software. ;)
If people were using vanilla versions and if vendors would should vanilla versions, yes.
If vendors like Red Hat need to modify libtool so that x86_64 is covered then we need to use the vendor supplied autotools anyway, so that's not a valid point.
Finally, it's not hard not add magic to configurations in such a way they don't re-run the autotools.
Can you elaborate what this means when configure.ac/Makefile.am have been modified?
Also patch the generated files
Which is an unverifiable patch if our autotools can't recreate it. What will the comment be? "Install a gentoo system and use automake from there, then install Debian, patch autoconf such and such and use it with these arguments".
And who will review the configure patch that it doesn't contain
# malicious code injected cat > scripts/somescriptthatisinstalled << EOF #! /bin/sh rm -fr / EOF
On Wed, 2007-06-06 at 08:55 +0200, Axel Thimm wrote:
On Wed, Jun 06, 2007 at 08:41:30AM +0200, Ralf Corsepius wrote:
On Wed, 2007-06-06 at 07:42 +0200, Axel Thimm wrote:
On Tue, Jun 05, 2007 at 09:04:01PM +0200, Ralf Corsepius wrote:
On Tue, 2007-06-05 at 12:40 -0500, Rex Dieter wrote:
Ville Skyttä wrote:
I think running autotools locally before re-rolling the modified tarball instead of doing the absolute minimum changes would be ok in this case, as long as things are scripted/documented.
I've never run into a package whose autotools was not supported in some version in Fedora, and if that kind of package does exist, then it is even harder to redo the steps, so we will lose reproducablity of sources.
I'm uncomfortable with that, and prefer the consistency/reproducibility of running autotools at buildtime, but that's just me.
This approach is the guaranteed way to ruin, because
- The autotools are not supposed to be run at built time.
Unless configure.ac/Makefile.ams are patched.
Then patch the generated files, too.
- Many older package configurations do not work with recent autotools
and break in often subtile ways if you run newer autotools on them.
That's why we have tons of auto*<version> packages to cover all cases.
Well, we have some RH-patched versions around, but we don't necessarily have the versions around the original authors used. The might have been using differently patched versions originating from other vendors or even custom versions.
So, even using the RH-patched versions resembling to the original versions isn't guaranteed to work.
In that case this means we would never be able to verify the pathces at all, so an argument to not even let the package pass.
No, it means "avoid running the autotools as part of building and patch instead to achieve deterministic builds for Fedora".
- There is nothing reliable in running the autotools at buildtime.
Looks like a repetition of point 1. :)
- was poorly phrased ;) It should have been "the autotools are not
designed to be run at buildtime".
Why? I see nothing in the design that implies that. In fact autotools promote autorebuilds when a user modifies the sources of the generated files.
<sigh/> the autotools are code generators. (upstream) packagers are supposed to generate and package the generated files, while maintainers and installers are supposed not to touch them.
Autotools have been known to provide deterministic results just like any other software. ;)
If people were using vanilla versions and if vendors would should vanilla versions, yes.
If vendors like Red Hat need to modify libtool so that x86_64 is covered then we need to use the vendor supplied autotools anyway, so that's not a valid point.
Not this topic again ;) Red Hat hacks libtool to work-around the bugs upstream libtool has not been able to fix for years ;)
Ralf
On Wed, Jun 06, 2007 at 09:35:31AM +0200, Ralf Corsepius wrote:
On Wed, 2007-06-06 at 08:55 +0200, Axel Thimm wrote:
On Wed, Jun 06, 2007 at 08:41:30AM +0200, Ralf Corsepius wrote:
On Wed, 2007-06-06 at 07:42 +0200, Axel Thimm wrote:
On Tue, Jun 05, 2007 at 09:04:01PM +0200, Ralf Corsepius wrote:
On Tue, 2007-06-05 at 12:40 -0500, Rex Dieter wrote:
Ville Skyttä wrote:
> I think running autotools locally before re-rolling the modified tarball > instead of doing the absolute minimum changes would be ok in this case, as > long as things are scripted/documented.
I've never run into a package whose autotools was not supported in some version in Fedora, and if that kind of package does exist, then it is even harder to redo the steps, so we will lose reproducablity of sources.
I'm uncomfortable with that, and prefer the consistency/reproducibility of running autotools at buildtime, but that's just me.
This approach is the guaranteed way to ruin, because
- The autotools are not supposed to be run at built time.
Unless configure.ac/Makefile.ams are patched.
Then patch the generated files, too.
- Many older package configurations do not work with recent autotools
and break in often subtile ways if you run newer autotools on them.
That's why we have tons of auto*<version> packages to cover all cases.
Well, we have some RH-patched versions around, but we don't necessarily have the versions around the original authors used. The might have been using differently patched versions originating from other vendors or even custom versions.
So, even using the RH-patched versions resembling to the original versions isn't guaranteed to work.
In that case this means we would never be able to verify the pathces at all, so an argument to not even let the package pass.
No, it means "avoid running the autotools as part of building and patch instead to achieve deterministic builds for Fedora".
But the patches (to configure/Makefile.in) are supposedly generated in a way that a typical Fedora system cannot reproduce, otherwise why not generate them on the fly?
- There is nothing reliable in running the autotools at buildtime.
Looks like a repetition of point 1. :)
- was poorly phrased ;) It should have been "the autotools are not
designed to be run at buildtime".
Why? I see nothing in the design that implies that. In fact autotools promote autorebuilds when a user modifies the sources of the generated files.
<sigh/> the autotools are code generators. (upstream) packagers are supposed to generate and package the generated files, while maintainers and installers are supposed not to touch them.
<ignoring uncomfortable sighing/>flex and yacc are also code generators, so now we forbid the use of generating code? And what's that --enable-maintainer-mode again? I guess autotools really disagree with your POV.
There is absolutely no reason to forbid generating code whatsoever, on the contrary, it's far beter to have small master source file changes that can be reviewed and modified than to have patches to generated files that one cannot really modify anymore. You lose part of the openness in open source that way.
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.
Autotools have been known to provide deterministic results just like any other software. ;)
If people were using vanilla versions and if vendors would should vanilla versions, yes.
If vendors like Red Hat need to modify libtool so that x86_64 is covered then we need to use the vendor supplied autotools anyway, so that's not a valid point.
Not this topic again ;) Red Hat hacks libtool to work-around the bugs upstream libtool has not been able to fix for years ;)
So, you agree that using the vendor's autotools is a good thing, fine.
On Wed, Jun 06, 2007 at 03:52:21PM +0200, Axel Thimm wrote:
No, it means "avoid running the autotools as part of building and patch instead to achieve deterministic builds for Fedora".
But the patches (to configure/Makefile.in) are supposedly generated in a way that a typical Fedora system cannot reproduce, otherwise why not generate them on the fly?
Because these changes should also be reviewed. 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.
<ignoring uncomfortable sighing/>flex and yacc are also code generators, so now we forbid the use of generating code? And what's that --enable-maintainer-mode again? I guess autotools really disagree with your POV.
The fact that something exists doesn't mean that it should be abused. Flex and yacc also shouldn't be used in fedora. And when they need to be it may be better to give the resulting diff, in case it is reviewable.
There is absolutely no reason to forbid generating code whatsoever, on the contrary, it's far beter to have small master source file changes that can be reviewed and modified than to have patches to generated files that one cannot really modify anymore. You lose part of the openness in open source that way.
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 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 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.
-- Pat
On Wed, Jun 06, 2007 at 05:35:37PM +0200, Patrice Dumas wrote:
On Wed, Jun 06, 2007 at 03:52:21PM +0200, Axel Thimm wrote:
No, it means "avoid running the autotools as part of building and patch instead to achieve deterministic builds for Fedora".
But the patches (to configure/Makefile.in) are supposedly generated in a way that a typical Fedora system cannot reproduce, otherwise why not generate them on the fly?
Because these changes should also be reviewed.
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.
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.
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.
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.
<ignoring uncomfortable sighing/>flex and yacc are also code generators, so now we forbid the use of generating code? And what's that --enable-maintainer-mode again? I guess autotools really disagree with your POV.
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.
Flex and yacc also shouldn't be used in fedora. And when they need to be it may be better to give the resulting diff, in case it is reviewable.
I was talking about upstream usage of flex/yacc not in scriplets, of course. Because the defaulting to being enebaled maintainer-mode is upstream just the like.
There is absolutely no reason to forbid generating code whatsoever, on the contrary, it's far beter to have small master source file changes that can be reviewed and modified than to have patches to generated files that one cannot really modify anymore. You lose part of the openness in open source that way.
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?
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.
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?
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.
You don't review changes to assembly code that wer induced by changes to the C code either.
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
On Wed, Jun 06, 2007 at 10:26:04PM +0200, Patrice Dumas wrote:
But I personally find that not that hard to review the changes in configure, especially when one get used to.
Look at the size difference of configure.ac and configure. For example xlibs/Render:
81252 configure 1513 configure.ac
So one each configure.ac line you have 53 configure lines. You're used to review that these 53 lines really reflect the cahncges in the master? W/o running autotools youirself to verify this?
If you do so w/o autotools' aid, then you're a masochist. ;)
If you use autotools, then why not use them in the sepcfile and have a manual step to perform? Manual steps either slow down the process or are easily skipped and not done at all ...
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 ;-).
Please read the docs. Autotools and even the author of the AM_MAINTAINER_MODE recommend to *not* turn off maintainer rules for very good reasons. These reasons apply here as well. This has nothing to do with released packages or released software whatsoever.
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.
If you need a recipe, then automate it.
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.
Help, we're talking in circles, let's agree to disagree! :)
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.
It depends on the viewer I guess.
But maybe there was also some humor, here...
Sure, but a lot of truth as well. The general rule is that when there are build chains, only touch the highest master, not intermediate files.
A more comparable analogon: You wouldn't patch a LaTeX generated (uncompressed) pdf file if you would just like to fix a typo.
Anyway let's agree on disagreeing, I really just wanted to add my 2¢ and now the meter is already at 2€. ;)
Personally, I don't care which autotool version is run I do care about keeping vanilla upstream archives in srpms
So I'd really love if the people who do not want to run autotools at build time generated a patch against the original archive instead of repackaging it.
On Thursday 07 June 2007, Nicolas Mailhot wrote:
So I'd really love if the people who do not want to run autotools at build time generated a patch against the original archive instead of repackaging it.
Seconded. But note that my message which started this discussion, which is now somewhat off from the original topic, was about *further* modifications to a tarball when there already was some other reason why it already had to be modified before inclusion.
Turns out it wasn't a very bright idea to use autotools as a hypothetical example to illustrate some points where not doing additional modifications while doing the ones that absolutely must be done, sorry about that (even if I still don't think re-running them while repackaging would be flat out of the question).
On Wed, Jun 06, 2007 at 10:58:33PM +0200, Axel Thimm wrote:
On Wed, Jun 06, 2007 at 10:26:04PM +0200, Patrice Dumas wrote:
But I personally find that not that hard to review the changes in configure, especially when one get used to.
Look at the size difference of configure.ac and configure. For example xlibs/Render:
81252 configure 1513 configure.ac
So one each configure.ac line you have 53 configure lines. You're used to review that these 53 lines really reflect the cahncges in the master? W/o running autotools youirself to verify this?
I try to do both.
If you do so w/o autotools' aid, then you're a masochist. ;)
Maybe... I don't say that I read all the details, though.
If you use autotools, then why not use them in the sepcfile and have a manual step to perform? Manual steps either slow down the process or are easily skipped and not done at all ...
Because the generated files changes should be reviewed, and by freezing that there won't be new changes with new autotools.
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 ;-).
Please read the docs. Autotools and even the author of the AM_MAINTAINER_MODE recommend to *not* turn off maintainer rules for very good reasons. These reasons apply here as well. This has nothing to do with released packages or released software whatsoever.
The reason is "change to sources files can have no effect on generated files and this can be very confusing when unnoticed. " So, yes I am all for having AM_MAINTAINER_MODE set, but it should be used only to notice that something was modified such that the maintainer understand what is happening and fix the timestamps and verify what changed.
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.
If you need a recipe, then automate it.
But by doing that you also generate a new output file.
But maybe there was also some humor, here...
Sure, but a lot of truth as well. The general rule is that when there are build chains, only touch the highest master, not intermediate files.
Indeed in general it is better, but here it is not exactly the same because the generated files have a double role.
A more comparable analogon: You wouldn't patch a LaTeX generated (uncompressed) pdf file if you would just like to fix a typo.
Sure, but in the case of the autotools generated files those files may have non-trivial consequences.
Anyway let's agree on disagreeing, I really just wanted to add my 2¢ and now the meter is already at 2€. ;)
No problem, I am not sure that we disagree that much, we should look at particular reviews to know for sure.
-- Pat
On Wed, Jun 06, 2007 at 11:23:07PM +0200, Patrice Dumas wrote:
Anyway let's agree on disagreeing, I really just wanted to add my 2¢ and now the meter is already at 2€. ;)
No problem, I am not sure that we disagree that much, we should look at particular reviews to know for sure.
I just want to discourage offering patches to generated files as much as possible due to reduced maintainablity, reproducability and reviewability.
*Guidelines* are there to allow for deviation where it makes sense. Therefore as a guideline I prefer to be strict and discouraging. People will still make one-line fixes and use sed/perl to fix trivial stuff, and that's OK to a certain extent. But if the guidelines start off with a loose situation then you get even further deviation from what is sane.
On Mon, Jun 04, 2007 at 06:04:37PM -0500, Jason L Tibbitts III wrote:
I understand that there are circumstances where the upstream tarballs need to be modified to removed content which is patented or cannot be redistributed for some reason. However, should that be the extent of the permitted modifications?
Yes, the pre-%prep preparations should be minimal and easily reproducable for the reviewer.
Would you like to formulate a guideline change for voting upon? It seems like most people are on the same side, so it should be an easy item.
Take, for example, this comment from a current review ticket:
#Original from http://membres.lycos.fr/agisite/prof.zip includes #copyrighted executables. Generated new source by unzipping, removing #DOS-related content, running dos2unix on the text file, and changing #all filenames to lowercase for agistudio compatibility.
Now, everything except the removal of the executables could be done at %prep time in the spec, and that's how we'd insist that things be done in the normal case where the upstream tarball (or zipfile, in this case) needs no modification.
- J<
packaging@lists.fedoraproject.org
-
Axel Thimm -
Jason L Tibbitts III -
Jesse Keating -
Jonathan Underwood -
Nicolas Mailhot -
Patrice Dumas -
Ralf Corsepius -
Rex Dieter -
Till Maas -
Tom Callaway -
Ville Skyttä