Should AM_MAINTAINER_MODE just be added to XORG_DEFAULT_OPTIONS then?

(BTW, from your list, pixman, mesa/*, xcb/*, and xkeyboard-config are sister
 projects we work closely with and need to have a full X desktop, but
 which X.Org doesn't control nor impose our configuration standards on.
 They'll probably accept patches for AM_MAINTAINER_MODE, but I don't know if
 they want to depend on xorg-macros.)

(In reply to comment #1)
> Should AM_MAINTAINER_MODE just be added to XORG_DEFAULT_OPTIONS then?
> 
> (BTW, from your list, pixman, mesa/*, xcb/*, and xkeyboard-config are sister
>  projects we work closely with and need to have a full X desktop, but
>  which X.Org doesn't control nor impose our configuration standards on.
>  They'll probably accept patches for AM_MAINTAINER_MODE, but I don't know if
>  they want to depend on xorg-macros.)
> 

Thanks, I meant to ask about the relationship between xorg and those for a while now. I sort of figure it out by observing. I had not noticed the sister projects were not using XORG_ macros! This clarifies the boundary.

All 251 xorg/* modules have been reviewed. Patches have been created locally and will be released soon.

All patches applied.

(In reply to comment #0)
> This mode is required by default to turn off maintainer build rules.
>...

Is there a rationale *why* it is "required by default" in X.Org?

I don't claim to be a frequent X.Org contributor, but when doing patches like #25104 discovering after some time that AM_MAINTAINER_MODE has disabled the automatic rebuild you were relying on is not nice.

> --- Comment #5 from Adrian Bunk <bunk@stusta.de>  2009-12-02 15:03:36 PST ---
> (In reply to comment #0)
> > This mode is required by default to turn off maintainer build rules.
> >...
> 
> Is there a rationale *why* it is "required by default" in X.Org?
> 
Because that's the sane thing to do for tarball builds, where the user
might not have the autotools.  Note that autogen.sh does set
--enable-maintainer-mode.

(In reply to comment #6)
> > --- Comment #5 from Adrian Bunk <bunk@stusta.de>  2009-12-02 15:03:36 PST ---
> > (In reply to comment #0)
> > > This mode is required by default to turn off maintainer build rules.
> > >...
> > 
> > Is there a rationale *why* it is "required by default" in X.Org?
> > 
> Because that's the sane thing to do for tarball builds, where the user
> might not have the autotools.  Note that autogen.sh does set
> --enable-maintainer-mode.

When are you running into any problems with tarball builds without it?

(In reply to comment #7)
> (In reply to comment #6)
> > > --- Comment #5 from Adrian Bunk <bunk@stusta.de>  2009-12-02 15:03:36 PST ---
> > > (In reply to comment #0)
> > > > This mode is required by default to turn off maintainer build rules.
> > > >...
> > > 
> > > Is there a rationale *why* it is "required by default" in X.Org?
> > > 
> > Because that's the sane thing to do for tarball builds, where the user
> > might not have the autotools.  Note that autogen.sh does set
> > --enable-maintainer-mode.
> 
> When are you running into any problems with tarball builds without it?
> 

If I understand the question correctly, was the addition of AM_MAINTAINER_MODE done because of some problems?

Opinions will vary on this. Before my involvement, a choice was made to enable maintainer-clean target in makefile, but only when building from git and not in a tarball. Reason being is that from a tarball (typically distro builders or installers), tools may be missing to recreate some files (e.g. ChangeLog which is generated from git). It's a protection, but one can run ./configure --enable-maintainer-mode to override.

For this feature to work as intended, 3 pieces are needed:
1) autogen.sh to use --enable-maintainer-mode
2) configure.ac to have AM_MAINTAINER_MODE
3) MAINTAINERCLEANFILES in Makefile.am

As time went by, I suppose, some lines were removed or not added, whatever. The whole thing was inconsistent. I just re-instated what was originally intended.

Technically, if you don't have AM_MAINTAINER_MODE, you get a warning (autoreconf -vfiWall) due to unknown --enable-maintainer-mode option. 

I have written something in the wiki http://wiki.x.org/wiki/NewModuleGuidelines.

I am curious as why you are asking, has this caused you any trouble? I like to hear from various people on they use things. This is just a default behaviour, one can still do anything he wants.

(In reply to comment #5)
> (In reply to comment #0)
> > This mode is required by default to turn off maintainer build rules.
> >...
> 
> Is there a rationale *why* it is "required by default" in X.Org?
> 
> I don't claim to be a frequent X.Org contributor, but when doing patches like
> #25104 discovering after some time that AM_MAINTAINER_MODE has disabled the
> automatic rebuild you were relying on is not nice.
> 

I am sorry, I had missed your original comment. I explained what the function is about. I don't understand what you mean by "automatic rebuild", can you elaborate? 

The only thing that you can no longer do by default is:

make maintainer-clean;

I can't see how it is related to rebuilding. If you tell me which xorg module you are working on (and which branch), I will verify that this function is working correctly and that nothing else has been changed accidentally.

(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #6)
> > > > --- Comment #5 from Adrian Bunk <bunk@stusta.de>  2009-12-02 15:03:36 PST ---
> > > > (In reply to comment #0)
> > > > > This mode is required by default to turn off maintainer build rules.
> > > > >...
> > > > 
> > > > Is there a rationale *why* it is "required by default" in X.Org?
> > > > 
> > > Because that's the sane thing to do for tarball builds, where the user
> > > might not have the autotools.  Note that autogen.sh does set
> > > --enable-maintainer-mode.
> > 
> > When are you running into any problems with tarball builds without it?
> > 
> 
> If I understand the question correctly, was the addition of AM_MAINTAINER_MODE
> done because of some problems?

Yes.

> Opinions will vary on this. Before my involvement, a choice was made to enable
> maintainer-clean target in makefile, but only when building from git and not in
> a tarball. Reason being is that from a tarball (typically distro builders or
> installers), tools may be missing to recreate some files (e.g. ChangeLog which
> is generated from git). It's a protection, but one can run ./configure
> --enable-maintainer-mode to override.
>...

I am talking about AM_MAINTAINER_MODE, and you are talking about maintainer-clean.

They are unrelated.

And I just tested in xwininfo and libXft that AM_MAINTAINER_MODE does not protect against removing the ChangeLog with "make maintainer-clean".

> 
> I am talking about AM_MAINTAINER_MODE, and you are talking about
> maintainer-clean.
> 
> They are unrelated.
They should be related. See the end of this comment with the explanation from Automake manual.
> 
> And I just tested in xwininfo and libXft that AM_MAINTAINER_MODE does not
> protect against removing the ChangeLog with "make maintainer-clean".
> 

I tested in xclock and the maintainer-clean target is available by default, which is wrong. When running ./configure, observe the message:
checking whether to enable maintainer-specific portions of Makefiles... no
It says no, but it does it anyway. I'll have to investigate.

autogen.sh calls ./configure with --enable-mainatiner-mode so you cannot use that to test it. Use:
autoreconf -vfi
./configure --prefix...

But this is not what you were concerned with in the first place. Something went wrong in your "rebuilding" that I still don't know anything about. 

-------------------------------------------------------------------
From Automake manual:

AM_MAINTAINER_MODE

AM_MAINTAINER_MODE allows you to choose whether the so called "rebuild rules" should be enabled or disabled. With AM_MAINTAINER_MODE([enable]), they are enabled by default, otherwise they are disabled by default. In the latter case, if you have AM_MAINTAINER_MODE in configure.ac, and run ‘./configure && make’, then make will *never* attempt to rebuild configure, Makefile.ins, Lex or Yacc outputs, etc. I.e., this disables build rules for files that are usually distributed and that users should normally not have to update.

The user can override the default setting by passing either ‘--enable-maintainer-mode’ or ‘--disable-maintainer-mode’ to configure.

People use AM_MAINTAINER_MODE either because they do not want their users (or themselves) annoyed by timestamps lossage (see CVS), or because they simply can't stand the rebuild rules and prefer running maintainer tools explicitly.

AM_MAINTAINER_MODE also allows you to disable some custom build rules conditionally. Some developers use this feature to disable rules that need exotic tools that users may not have available.

Several years ago François Pinard pointed out several arguments against this AM_MAINTAINER_MODE macro. Most of them relate to insecurity. By removing dependencies you get non-dependable builds: changes to sources files can have no effect on generated files and this can be very confusing when unnoticed. He adds that security shouldn't be reserved to maintainers (what --enable-maintainer-mode suggests), on the contrary. If one user has to modify a Makefile.am, then either Makefile.in should be updated or a warning should be output (this is what Automake uses missing for) but the last thing you want is that nothing happens and the user doesn't notice it (this is what happens when rebuild rules are disabled by AM_MAINTAINER_MODE).

Jim Meyering, the inventor of the AM_MAINTAINER_MODE macro was swayed by François's arguments, and got rid of AM_MAINTAINER_MODE in all of his packages.

Still many people continue to use AM_MAINTAINER_MODE, because it helps them working on projects where all files are kept under CVS, and because missing isn't enough if you have the wrong version of the tools. 

---------------------------------------------------------------------------------

(In reply to comment #11)
> > 
> > I am talking about AM_MAINTAINER_MODE, and you are talking about
> > maintainer-clean.
> > 
> > They are unrelated.
> They should be related. See the end of this comment with the explanation from
> Automake manual.

I know the automake manual.

I also looked at automake's sourcecode.

And they are not.

> > And I just tested in xwininfo and libXft that AM_MAINTAINER_MODE does not
> > protect against removing the ChangeLog with "make maintainer-clean".
> > 
> 
> I tested in xclock and the maintainer-clean target is available by default,
> which is wrong. When running ./configure, observe the message:
> checking whether to enable maintainer-specific portions of Makefiles... no
> It says no, but it does it anyway. I'll have to investigate.

The "maintainer-specific portions of Makefiles" are the rebuild rules AM_MAINTAINER_MODE disables.

This has nothing to do with maintainer-clean.

> autogen.sh calls ./configure with --enable-mainatiner-mode so you cannot use
> that to test it. Use:
> autoreconf -vfi
> ./configure --prefix...

I am not that stupid.

What I did was:

./autogen.sh
./configure
make ChangeLog
ls ChangeLog
make maintainer-clean
ls ChangeLog


How did you verify that your changes do succeed in disabling maintainer-clean before committing your patches?


> But this is not what you were concerned with in the first place. Something went
> wrong in your "rebuilding" that I still don't know anything about. 

After manually editing configure.ac (something a normal user would never do) the configure file did not get rebuilt automatically.

Disabling these rebuild rules is exactly the one and only thing AM_MAINTAINER_MODE does.

> -------------------------------------------------------------------
> From Automake manual:
> 
> AM_MAINTAINER_MODE
>...

The whole text you quoted does not contain the string "clean" at all...


I'm reopening this bug since your changes did not fix the problem you wanted to fix.

(In reply to comment #12)

I did try a small test case and the maintainer-clean target is always available as you mention. This comes as a surprise to me as I do remember having testing this. 

This is the phrase in the text that lead me to this scenario:
"AM_MAINTAINER_MODE also allows you to disable some custom build rules conditionally. Some developers use this feature to disable rules that need exotic tools that users may not have available."

It looks like I made a connection where none should have been made.

Let me think about it some more. Right now, I don't think it changes anything. The reviewers really wanted to have MAINTAINERCLEANFILES for ChangeLog and INSTALL, but I received no comments regarding AM_MAINTAINER_MODE. They were also not concerned about preventing accidental invocation of maintainer-clean target. I noticed Debian distro recommends it while I saw some others who removed it. 

Thanks for raising this, I am sorry I did not understand you the first time.

(In reply to comment #13)
>...
> Let me think about it some more. Right now, I don't think it changes anything.
> The reviewers really wanted to have MAINTAINERCLEANFILES for ChangeLog and
> INSTALL, but I received no comments regarding AM_MAINTAINER_MODE. They were
> also not concerned about preventing accidental invocation of maintainer-clean
> target. I noticed Debian distro recommends it while I saw some others who
> removed it. 

Debian had 10 years ago the problem that there was no proper patch management, and all changes to the upstream sources were in one big diff.

In that situation AM_MAINTAINER_MODE can help.

Debian now has a proper patch management that can handle this better.

> Thanks for raising this, I am sorry I did not understand you the first time.
 
No problem.

What's the status of this?  I'm a tad confused.  Why don't we just have this 
*once* in XORG_DEFAULT_OPTIONS?

(In reply to comment #15)
> What's the status of this?  I'm a tad confused.  Why don't we just have this 
> *once* in XORG_DEFAULT_OPTIONS?

The first question is if AM_MAINTAINER_MODE is considered a good idea or not.

Opinions on that vary, and as the automake info page explains even the inventor of AM_MAINTAINER_MODE was convinced to no longer use it...

I am one of the people who *hate* it (not that my word had much value in the X community), read see my discussion with Gaetan below for details.

If AM_MAINTAINER_MODE is wanted in X you are right, and your "once in XORG_DEFAULT_OPTIONS" would be a good approach.

Please double-check in this case that specifying AM_MAINTAINER_MODE twice when mixing older components with AM_MAINTAINER_MODE and updated macros is not causing any problems (I'd expect it to work).

(In reply to comment #16)
> (In reply to comment #15)
> > What's the status of this?  I'm a tad confused.  Why don't we just have this 
> > *once* in XORG_DEFAULT_OPTIONS?
> 
> The first question is if AM_MAINTAINER_MODE is considered a good idea or not.
> 
> Opinions on that vary, and as the automake info page explains even the inventor
> of AM_MAINTAINER_MODE was convinced to no longer use it...
> 
> I am one of the people who *hate* it (not that my word had much value in the X
> community), read see my discussion with Gaetan below for details.
> 
> If AM_MAINTAINER_MODE is wanted in X you are right, and your "once in
> XORG_DEFAULT_OPTIONS" would be a good approach.
> 
> Please double-check in this case that specifying AM_MAINTAINER_MODE twice when
> mixing older components with AM_MAINTAINER_MODE and updated macros is not
> causing any problems (I'd expect it to work).

I agree with Adrian, after a long and painful learning curve. The original thought was to have all modules consistent, no reasons to have some with and some without. This is done now, they all have it.

I am the one who reported the problem and I consider it fixed and anyone can verify the fix with some minor grepping.

This leaves the other issue that AM_MAINTAINER is not desirable simply because it is a CVS workaround, a solution to a problem we do not have. This is a big job, as it requires a patch to configure.ac and autogen.sh on each of the 240 modules.

This work will eventually be done through patch review on xorg-devel.