Modifying upstream tarballs

[Jason L Tibbitts III]

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<

[Jonathan Underwood]

On 04 Jun 2007 18:04:37 -0500, Jason L Tibbitts III <tibbs(a)math.uh.edu&gt; 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?

[Jason L Tibbitts III]

> >>>> "JU" == Jonathan Underwood
> <jonathan.underwood(a)gmail.com&gt; 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<

[Jesse Keating]

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.

-- 
Jesse Keating
Release Engineer: Fedora

[Jesse Keating]

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.

-- 
Jesse Keating
Release Engineer: Fedora

[Tom Callaway]

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

[Till Maas]

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

[Tom Callaway]

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

[Ville Skyttä]

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.

[Jason L Tibbitts III]

> >>>> "VS" == Ville Skyttä
> <ville.skytta(a)iki.fi&gt; 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<

[Ville Skyttä]

On Tuesday 05 June 2007, Jason L Tibbitts III wrote:
>  >>>>> "VS" == Ville Skyttä
> <ville.skytta(a)iki.fi&gt; 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.

[Rex Dieter]

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

[Ralf Corsepius]

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

[Axel Thimm]

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
>  
>  1. The autotools are not supposed to be run at built time. 
Unless configure.ac/Makefile.ams are patched.

>  2. 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.

>  3. 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.
-- 
Axel.Thimm at ATrpms.net

[Ralf Corsepius]

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
>  > 
>  > 1. The autotools are not supposed to be run at built time.
>  
>  Unless configure.ac/Makefile.ams are patched. 
Then patch the generated files, too.


>  > 2. 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. 

>  > 3. 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

[Axel Thimm]

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
>  > > 
>  > > 1. The autotools are not supposed to be run at built time.
>  > 
>  > Unless configure.ac/Makefile.ams are patched.
>  Then patch the generated files, too. 
>  
>  > > 2. 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.

>  > > 3. 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". 
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
-- 
Axel.Thimm at ATrpms.net

[Ralf Corsepius]

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
>  > > > 
>  > > > 1. The autotools are not supposed to be run at built time.
>  > > 
>  > > Unless configure.ac/Makefile.ams are patched.
>  > Then patch the generated files, too. 
>  > 
>  > > > 2. 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".

>  > > > 3. 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".
>  
>  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

[Axel Thimm]

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
>  > > > > 
>  > > > > 1. The autotools are not supposed to be run at built time.
>  > > > 
>  > > > Unless configure.ac/Makefile.ams are patched.
>  > > Then patch the generated files, too. 
>  > > 
>  > > > > 2. 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?

>  > > > > 3. 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".
>  > 
>  > 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.
-- 
Axel.Thimm at ATrpms.net

[Patrice Dumas]

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

[Axel Thimm]

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.
-- 
Axel.Thimm at ATrpms.net

[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

[Axel Thimm]

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€. ;)
-- 
Axel.Thimm at ATrpms.net

[Nicolas Mailhot]

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.

-- 
Nicolas Mailhot

[Ville Skyttä]

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).

[Patrice Dumas]

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

[Axel Thimm]

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.
-- 
Axel.Thimm at ATrpms.net

[Axel Thimm]

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<
>   
-- 
Axel.Thimm at ATrpms.net