Witness computation - minor breakage in the build system

Witness computation - minor breakage in the build system

[Eric S. Raymond]

Is any particular person responsible for the build system?

While working on one of my to-do items I found an apparent bug
in Makefile.in.  The src production tries to compute the location 
of a "witness" file with modtime taken as the time of most recent
repository change.

The computation is wrong, however, and fails - leaving VCSWITNESS 
empty.  This has escaped notice because the effect is subtle: 
re-dumping may not always be forced when it should be after a
repository update.  To see that this true, add code just before
the make invocation on line 384 to dump $vcswitness.  Don't forget
to double the $.

I stumbled over this because part of my transition plan is to get
the code to a state where it can do a full build (with proper revision
ID amd conditional dumping) regardless of whether the underlying 
repo is Bazaar or Git.

I could patch-fix this.  But I think the fixed Makefile code would
remain unpleasantly fragile, and the witness-path computation is
best moved to the configure stage.

Comments? Objections? Is anyone else willing to take respomsibility
for this?
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

Democracy is two wolves and a lamb voting on what to have for lunch. Liberty is
a well-armed lamb contesting the vote!
	-- Benjamin Franklin

Re: Witness computation - minor breakage in the build system

[Paul Eggert]

Eric S. Raymond wrote:
> I think the fixed Makefile code would
> remain unpleasantly fragile, and the witness-path computation is
> best moved to the configure stage.

I've never understood the need for having the
last-repository-change date affect the build, and
would favor removing all the vcswitness stuff.

> Is any particular person responsible for the build system?

No, though Glenn and I probably fiddle with it the most.

Re: Witness computation - minor breakage in the build system

[Eric S. Raymond]

Paul Eggert <eggert@cs.ucla.edu>:
> Eric S. Raymond wrote:
> >I think the fixed Makefile code would
> >remain unpleasantly fragile, and the witness-path computation is
> >best moved to the configure stage.
> 
> I've never understood the need for having the
> last-repository-change date affect the build, and
> would favor removing all the vcswitness stuff.

Here's what the src/Makefile.in says about it:

## VCSWITNESS points to the file that holds info about the current checkout.
## We use it as a heuristic to decide when to rebuild loaddefs.el.
## If empty it is ignored; the parent makefile can set it to some other value.

It would make my life simpler if this mechanism were removed; that's one of only
two dependencies of Cazaar in the entire build (the other one is in 
lisp/version.el).  And the fact that the bug went undetected suggests that
the mechanism wasn't very useful.

Anyone want to argue for keeping this?  Support removing it?  Suggest
something else?
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

Re: Witness computation - minor breakage in the build system

[Stefan Monnier]

>> I think the fixed Makefile code would remain unpleasantly fragile,
>> and the witness-path computation is best moved to the
>> configure stage.
> I've never understood the need for having the last-repository-change
> date affect the build, and would favor removing all the
> vcswitness stuff.

Simple:
- "make autoloads" causes an undesirable re-dump even when no autoload changed.
- for that reason "autoloads" is not in the dependencies and is not
  always run.

So the vcswitness is a completely opportunistic hack which forces
a "make autoloads" every once in a while.


        Stefan

Re: Witness computation - minor breakage in the build system

[Stefan Monnier]

> Anyone want to argue for keeping this?  Support removing it?  Suggest
> something else?

The comment makes it clear it's a "best effort" hack.  I see no benefit
to removing it.  If it works, great.  If it doesn't, oh well.


        Stefan

Re: Witness computation - minor breakage in the build system

[Eric S. Raymond]

Stefan Monnier <monnier@iro.umontreal.ca>:
> The comment makes it clear it's a "best effort" hack.  I see no benefit
> to removing it.  If it works, great.  If it doesn't, oh well.

Your mail crossed mine about removing it, sorry.

The benefit is that a deoendency on the VCS goes away.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

Re: Witness computation - minor breakage in the build system

[Eli Zaretskii]

> Date: Sat, 11 Jan 2014 20:50:02 -0800
> From: Paul Eggert <eggert@cs.ucla.edu>
> 
> Eric S. Raymond wrote:
> > I think the fixed Makefile code would
> > remain unpleasantly fragile, and the witness-path computation is
> > best moved to the configure stage.
> 
> I've never understood the need for having the
> last-repository-change date affect the build, and
> would favor removing all the vcswitness stuff.

How else would you trigger refresh of loaddefs.el?  Suggestions are
welcome.

Re: Witness computation - minor breakage in the build system

[Eli Zaretskii]

> Date: Sun, 12 Jan 2014 00:08:59 -0500
> From: "Eric S. Raymond" <esr@thyrsus.com>
> Cc: emacs-devel@gnu.org
> 
> ## VCSWITNESS points to the file that holds info about the current checkout.
> ## We use it as a heuristic to decide when to rebuild loaddefs.el.
> ## If empty it is ignored; the parent makefile can set it to some other value.
> 
> It would make my life simpler if this mechanism were removed; that's one of only
> two dependencies of Cazaar in the entire build (the other one is in 
> lisp/version.el).  And the fact that the bug went undetected suggests that
> the mechanism wasn't very useful.
> 
> Anyone want to argue for keeping this?  Support removing it?  Suggest
> something else?

We need something to refresh loaddefs.el from time to time.  If you
have alternative suggestions, please describe them.

Re: Witness computation - minor breakage in the build system

[Glenn Morris]

Paul Eggert wrote:

> No, though Glenn and I probably fiddle with it the most.

And personally speaking, I need more than 12 hours to be able to respond
to something. And waking up to find something has already been changed,
and discussion splatted across 3 different threads does not help.

I'm going to guess that the problem is that srcdir used to be absolute,
and is now relative. So maybe use abs_srcdir instead.
But I have had no time to look at the issue properly.

A clear bug report would be enormously useful.

Re: Witness computation - minor breakage in the build system

[Paul Eggert]

Eli Zaretskii wrote:
> How else would you trigger refresh of loaddefs.el?

I'd have it depend on the files that were last used
to generate loaddefs.el.  The list of such files
could be computed automatically, when generating
loaddefs.el, and the list could be shipped as part
of the Emacs tarball so that the process would work
even for people not using a version control system,
or who have imported Emacs into some other version
control system.

It shouldn't be that hard, though we should not
be installing this sort of change during a feature
freeze.

Re: Witness computation - minor breakage in the build system

[Stefan Monnier]

> The benefit is that a deoendency on the VCS goes away.

I don't see that should be considered as a benefit.


        Stefan

Re: Witness computation - minor breakage in the build system

[Eric S. Raymond]

Paul Eggert <eggert@cs.ucla.edu>:
> Eli Zaretskii wrote:
> >How else would you trigger refresh of loaddefs.el?
> 
> I'd have it depend on the files that were last used
> to generate loaddefs.el.  The list of such files
> could be computed automatically, when generating
> loaddefs.el, and the list could be shipped as part
> of the Emacs tarball so that the process would work
> even for people not using a version control system,
> or who have imported Emacs into some other version
> control system.
> 
> It shouldn't be that hard, though we should not
> be installing this sort of change during a feature
> freeze.

+1

To both the recommendation and the caveat that we
shouldn't do it now.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

Re: Witness computation - minor breakage in the build system

[David Kastrup]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> The benefit is that a deoendency on the VCS goes away.
>
> I don't see that should be considered as a benefit.

Well, the whole point of the GPL and the delivered source tar balls is
to give the user the ability to make his own changes.  If the Makefile
dependencies don't work when making changes to the source from a
tarball, it does not look much like a source tarball.

The GPL states regarding "corresponding source":

      The "Corresponding Source" for a work in object code form means all
    the source code needed to generate, install, and (for an executable
    work) run the object code and to modify the work, including scripts to
    control those activities.

If it is not possible to modify the work reasonably without having a
source code checkout available, then the tarball is not the
corresponding source according to the above definition as it does not
allow for modifying the work.

I'm not overly interested in what the lawyers have to say about this,
but in my opinion it at least concerns the _spirit_ of the GPL if what
we distribute as the source will not lend itself to modification and
recompilation.

-- 
David Kastrup

Re: Witness computation - minor breakage in the build system

[Andreas Schwab]

David Kastrup <dak@gnu.org> writes:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>>> The benefit is that a deoendency on the VCS goes away.
>>
>> I don't see that should be considered as a benefit.
>
> Well, the whole point of the GPL and the delivered source tar balls is
> to give the user the ability to make his own changes.  If the Makefile
> dependencies don't work when making changes to the source from a
> tarball, it does not look much like a source tarball.

I don't think you can put that much into it.  You can alway run `make -C
lisp autoloads' manually to update loaddefs.el.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Witness computation - minor breakage in the build system

[David Kastrup]

Andreas Schwab <schwab@linux-m68k.org> writes:

> David Kastrup <dak@gnu.org> writes:
>
>> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>
>>>> The benefit is that a deoendency on the VCS goes away.
>>>
>>> I don't see that should be considered as a benefit.
>>
>> Well, the whole point of the GPL and the delivered source tar balls is
>> to give the user the ability to make his own changes.  If the Makefile
>> dependencies don't work when making changes to the source from a
>> tarball, it does not look much like a source tarball.
>
> I don't think you can put that much into it.  You can alway run `make -C
> lisp autoloads' manually to update loaddefs.el.

Is that documented anywhere?  "You can always" is adequate justification
for not including any build script, since you can always execute all
steps manually.  If there is no reasonably discoverable way of what one
can always do, it seems somewhat theoretic.

The usual expectation is that modifying files will let "make" rebuild
what is necessary.

-- 
David Kastrup

Re: Witness computation - minor breakage in the build system

[Eli Zaretskii]

> Date: Sun, 12 Jan 2014 11:22:14 -0800
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: esr@thyrsus.com, emacs-devel@gnu.org
> 
> It shouldn't be that hard, though we should not
> be installing this sort of change during a feature
> freeze.

Then let's talk about that when such a change can be presented and
installed.

Thanks.

Re: Witness computation - minor breakage in the build system

[Stefan Monnier]

>> I'd have it depend on the files that were last used
>> to generate loaddefs.el.

You mean all of the lisp/**/*.el files?  Bad idea:
- it would fail to take into account new .el files (not a big deal, but
  still: we're still in the "best effort" category).
- it would cause re-dumping everytime you change a .el file, even tho in
  99% of the cases the resulting loaddefs.el is the same.


        Stefan

Re: Witness computation - minor breakage in the build system

[Paul Eggert]

Stefan Monnier wrote:
> You mean all of the lisp/**/*.el files?  Bad idea:
> - it would fail to take into account new .el files (not a big deal, but
>    still: we're still in the "best effort" category).

That's a problem with the current approach too, no?
If I create an .el file, it's ignored -- unless
I futz with the repository too.

So this sounds like more of a 'make bootstrap'ish issue to me.
However, if it turns into a real problem in ordinary development
we can add a dependency on the lisp/** directories
as well as on the .el files.

> - it would cause re-dumping everytime you change a .el file, even tho in
>    99% of the cases the resulting loaddefs.el is the same.

That shouldn't be a problem.
Undumping should depend on the witness file,
not on the .el files directly; and the witness shouldn't
change unless one of the autoloads actually changed.

Re: Witness computation - minor breakage in the build system

[Stefan Monnier]

> That shouldn't be a problem.
> Undumping should depend on the witness file,
> not on the .el files directly; and the witness shouldn't
> change unless one of the autoloads actually changed.

The witness file does change, tho.


        Stefan

Re: Witness computation - minor breakage in the build system

[Glenn Morris]

Eric S. Raymond wrote:

> Is any particular person responsible for the build system?

BTW, IMO there's never a need to ask this kind of question.
If you find a bug, make a bug report.
If you want to discuss something more general, send a mail to emacs-devel.
Then wait several days to allow any interested parties time to respond.
You may wish to cc: someone (eg any listed Maintainer: for the file in
question) to draw their attention, but it's not essential.
And if you want to know who wrote a bit of code, see the ChangeLog,
or vc-annotate.

Re: Witness computation - minor breakage in the build system

[Paul Eggert]

Stefan Monnier wrote:
> The witness file does change, tho.

Do you mean that its contents change, or that its timestamp changes?

If the witness file's contents depend only on the
set of autoloaded functions, and if its time stamp reflects
the last time its contents changed, we should get the
dependency that we want.

We'll also need a timestamp file, which will have the
last time the witness file was recomputed,
as opposed to the last time that it changed.
This timestamp file can depend on all the .el files,
whereas the later parts of the build procedure
can depend on the witness file.

Re: Witness computation - minor breakage in the build system

[Stefan Monnier]

>> The witness file does change, tho.
> Do you mean that its contents change, or that its timestamp changes?

Its contents change (at the very because its contents include
a timestamp of the last time it was built, and the time-stamp of each of
the **/*.el files). This content is in Elisp comments, so it doesn't
actually influence the dump.


        Stefan

Re: Witness computation - minor breakage in the build system

[Paul Eggert]

Stefan Monnier wrote:
> Its contents change (at the very because its contents include
> a timestamp of the last time it was built, and the time-stamp of each of
> the **/*.el files). This content is in Elisp comments, so it doesn't
> actually influence the dump.

Then we could ignore comments when determining whether the
contents have changed.

Re: Witness computation - minor breakage in the build system

[Stefan Monnier]

>> Its contents change (at the very because its contents include
>> a timestamp of the last time it was built, and the time-stamp of each of
>> the **/*.el files). This content is in Elisp comments, so it doesn't
>> actually influence the dump.
> Then we could ignore comments when determining whether the
> contents have changed.

Yes.


        Stefan