VCSWITNESS = fail ** 2

VCSWITNESS = fail ** 2

[Eric S. Raymond]

Nobody has stepped up to own or defend this code in Makefile.in:

src: Makefile FRC
	dirstate='.bzr/checkout/dirstate';				\
	vcswitness='$$(srcdir)/../'$$dirstate;				\
	[ -r "$(srcdir)/$$dirstate" ] || vcswitness='';			\
	cd $@ || exit;							\
	boot=bootstrap-emacs$(EXEEXT);					\
	[ ! -x "$$boot" ] || boot='';					\
	$(MAKE) all $(MFLAGS)						\
	  CC='${CC}' CFLAGS='${CFLAGS}' CPPFLAGS='${CPPFLAGS}'		\
	  LDFLAGS='${LDFLAGS}' MAKE='${MAKE}' BOOTSTRAPEMACS="$$boot"	\
	  VCSWITNESS="$$vcswitness"

Which is just as well, because the second through fourth lines

	dirstate='.bzr/checkout/dirstate';				\
	vcswitness='$$(srcdir)/../'$$dirstate;				\
	[ -r "$(srcdir)/$$dirstate" ] || vcswitness='';			\

are a case study in how not to do things.  First, they're broken.  The
readability test on line 4 is not checking the same file path that is
set on line 3. The contingent result with the current repo tree structure
is that vcswitness is never set. The necessary result under almost any
possible tree structure is that vcswitness would not be set correctly.

Now let's look at what VCSWITNESS is used for.  The key lines are
in src/Makefile.in:

## 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.
VCSWITNESS =

$(lispsource)/loaddefs.el: $(BOOTSTRAPEMACS) $(VCSWITNESS)
	cd ../lisp; $(MAKE) $(MFLAGS) autoloads EMACS="$(bootstrap_exe)"

This test is wrong.  The correct prerequisites would be every elisp
file that declares an autoload cookie.  According to 

find . -name '*.el' | xargs grep '#autoload' -l | wc -l

there are presently 786 such files.  The right thing would be for
configure to generate that list into a makefile variable referenced
where VCSWITNESS is now. 

I could find out who was responsible for this.  I think I'd prefer not
to know, and to hope the person was basically competent but having a
bad day.

I'm going to delete the broken code.  That will leave us no worse off,
and eliminate a Bazaar dependency.  If I weren't eyeball-deep in
preparations for the git transition I'd actually fix this.  But having
gone unnoticed for a long time suggests it's not very important. I'll
add an entry to BUGS or PROBLEMS.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

The difference between death and taxes is death doesn't get worse
every time Congress meets
	-- Will Rogers

Re: VCSWITNESS = fail ** 2

[Lars Magne Ingebrigtsen]

esr@thyrsus.com (Eric S. Raymond) writes:

> I'm going to delete the broken code.  That will leave us no worse off,
> and eliminate a Bazaar dependency.

It leaves us worse off, because autoloads will be generated even more
unreliably than today, as far as I can tell.

Again, you seem to not be fixing bugs, but doing irrelevant things
(prone to introducing new bugs, as when you removed the domain file from
etc) during the feature freeze.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: VCSWITNESS = fail ** 2

[Eric S. Raymond]

Lars Magne Ingebrigtsen <larsi@gnus.org>:
> esr@thyrsus.com (Eric S. Raymond) writes:
> 
> > I'm going to delete the broken code.  That will leave us no worse off,
> > and eliminate a Bazaar dependency.
> 
> It leaves us worse off, because autoloads will be generated even more
> unreliably than today, as far as I can tell.

The code I removed was a no-op. It *didn't work*. For all the good it was
doing us it might as well not have been there at all.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

Re: VCSWITNESS = fail ** 2

[Eli Zaretskii]

> From: esr@thyrsus.com (Eric S. Raymond)
> Date: Sun, 12 Jan 2014 09:37:33 -0500 (EST)
> 
> Nobody has stepped up to own or defend this code in Makefile.in:

You didn't wait nowhere near enough time to draw that conclusion.
Some of us live in different time zones than you do.  Moreover, for
some of us today is a weekday, not a weekend, and I, for instance,
cannot reply to my gnu.org email during office hours, for boring
reasons.

Please always wait for at least a day, if not longer, before you
conclude that no one responded.

> src: Makefile FRC
> 	dirstate='.bzr/checkout/dirstate';				\
> 	vcswitness='$$(srcdir)/../'$$dirstate;				\
> 	[ -r "$(srcdir)/$$dirstate" ] || vcswitness='';			\
> 	cd $@ || exit;							\
> 	boot=bootstrap-emacs$(EXEEXT);					\
> 	[ ! -x "$$boot" ] || boot='';					\
> 	$(MAKE) all $(MFLAGS)						\
> 	  CC='${CC}' CFLAGS='${CFLAGS}' CPPFLAGS='${CPPFLAGS}'		\
> 	  LDFLAGS='${LDFLAGS}' MAKE='${MAKE}' BOOTSTRAPEMACS="$$boot"	\
> 	  VCSWITNESS="$$vcswitness"
> 
> Which is just as well, because the second through fourth lines
> 
> 	dirstate='.bzr/checkout/dirstate';				\
> 	vcswitness='$$(srcdir)/../'$$dirstate;				\
> 	[ -r "$(srcdir)/$$dirstate" ] || vcswitness='';			\
> 
> are a case study in how not to do things.  First, they're broken.  The
> readability test on line 4 is not checking the same file path that is
> set on line 3.

AFAICS, that's because the test is done first from $(srcdir), and then
from $(srcdir)/src.  Thus, the value passed to src/Makefile needs an
extra "..".

> The contingent result with the current repo tree structure
> is that vcswitness is never set.

Isn't the problem in this line:

	  VCSWITNESS="$$vcswitness"

Shouldn't that be

	  VCSWITNESS="$vcswitness"

instead?

> Now let's look at what VCSWITNESS is used for.  The key lines are
> in src/Makefile.in:
> 
> ## 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.
> VCSWITNESS =
> 
> $(lispsource)/loaddefs.el: $(BOOTSTRAPEMACS) $(VCSWITNESS)
> 	cd ../lisp; $(MAKE) $(MFLAGS) autoloads EMACS="$(bootstrap_exe)"
> 
> This test is wrong.  The correct prerequisites would be every elisp
> file that declares an autoload cookie.

That possibility was considered and rejected, for the reasons that
Stefan explained.  So you have just made things worse, not better.

> I'm going to delete the broken code.

I'm going to revert that change, unless I hear some objections.  Then
we can discuss how to fix this (assuming it is broken, which I'm not
sure it is yet).

Re: VCSWITNESS = fail ** 2

[Eric S. Raymond]

Eli Zaretskii <eliz@gnu.org>:
> Shouldn't that be
> 
> 	  VCSWITNESS="$vcswitness"
> 
> instead?

No. You're in a Makefile context.  The $$ needs to be doubled or
it won't be passed to the shell.  I tested this.

> That possibility was considered and rejected, for the reasons that
> Stefan explained. 

I have not seen that explanation.

>                 So you have just made things worse, not better.

The code wasn't working, so deleting it didn't leave us any worse
off.  I did test before posting.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

Re: VCSWITNESS = fail ** 2

[Eli Zaretskii]

> Date: Sun, 12 Jan 2014 10:17:19 -0500
> From: "Eric S. Raymond" <esr@thyrsus.com>
> Cc: emacs-devel@gnu.org
> 
> Lars Magne Ingebrigtsen <larsi@gnus.org>:
> > esr@thyrsus.com (Eric S. Raymond) writes:
> > 
> > > I'm going to delete the broken code.  That will leave us no worse off,
> > > and eliminate a Bazaar dependency.
> > 
> > It leaves us worse off, because autoloads will be generated even more
> > unreliably than today, as far as I can tell.
> 
> The code I removed was a no-op. It *didn't work*.

It should work now, as of revision 115997.  Please check.

Re: VCSWITNESS = fail ** 2

[Eli Zaretskii]

> Date: Sun, 12 Jan 2014 11:45:20 -0500
> From: "Eric S. Raymond" <esr@thyrsus.com>
> Cc: emacs-devel@gnu.org
> 
> Eli Zaretskii <eliz@gnu.org>:
> > Shouldn't that be
> > 
> > 	  VCSWITNESS="$vcswitness"
> > 
> > instead?
> 
> No. You're in a Makefile context.  The $$ needs to be doubled or
> it won't be passed to the shell.  I tested this.

Right, the problem was elsewhere.  This fixed it, AFAICS:

-       vcswitness='$$(srcdir)/../'$$dirstate;                          \
+       vcswitness='$(srcdir)/../'$$dirstate;                           \

> > That possibility was considered and rejected, for the reasons that
> > Stefan explained. 
> 
> I have not seen that explanation.

Here it is:

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

> >                 So you have just made things worse, not better.
> 
> The code wasn't working, so deleting it didn't leave us any worse
> off.

The situation with refreshing loaddefs.el became worse, see above.

Re: VCSWITNESS = fail ** 2

[Eli Zaretskii]

> Date: Sun, 12 Jan 2014 19:36:20 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> Right, the problem was elsewhere.  This fixed it, AFAICS:
> 
> -       vcswitness='$$(srcdir)/../'$$dirstate;                          \
> +       vcswitness='$(srcdir)/../'$$dirstate;                           \

Actually, I take that back: the original code should work, and works
for me.

So let's please step back one notch: can you please tell what exactly
is wrong with the way vcswitness is computed and passed to the
sub-Make?

Re: VCSWITNESS = fail ** 2

[Eric S. Raymond]

Eli Zaretskii <eliz@gnu.org>:
> Actually, I take that back: the original code should work, and works
> for me.
> 
> So let's please step back one notch: can you please tell what exactly
> is wrong with the way vcswitness is computed and passed to the
> sub-Make?

What I know is this: 

I was trying to modify that code to DTRT in both a Bazaar and .git repo, 
using .git/HEAD as a witness in the latter case.  The way my modification 
failed caused me to suspect that the original code hadn't been working.

So I reverted to the trunk version and instrumented, by inserting an
echo before the generated make command.  Lo and behold, vcswitness was
empty.

I looked again.  Had I doubled $ properly for the make context?  I had;
vcswitness was really empty.  (But I was already pretty sure of that; if
doubling $$ had been incorrect it would have expanded as somebody's 
process number.)

I looked at those three lines again, saw the two different paths, and
made a deduction about the failure source that it now appears was 
incorrect.

But I've been around the track a few times, and I know that because 
of (a) conflicting Makefile and shell expansion rules  and (b) the
hair associated with nested Makefiles, this sort of code is notoriously
easy to misread (as Eli was later reminded himself).

So I posted to the list and waited for somebody to point out the flaw
in my reasoning.  Nobody did. Meanwhile, in my own test builds
vcswitness continued to be spuriously empty.

The rest you all know.  Eli took his own comic pratfall, momentarily
breaking the build. And he's been around the track a few times himself.

At present, I have no explanation for the observed differences in
behavior more predictive than "phase of the moon".

Those three lines are, by demonstration, so prone to be misread that they 
tripped up Eli and me in two different ways and I still have no theory
to account for why they were different.

That says "maintainence nightmare - should be scrapped and replaced"
to me.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

Re: VCSWITNESS = fail ** 2

[Eli Zaretskii]

> Date: Sun, 12 Jan 2014 13:45:23 -0500
> From: "Eric S. Raymond" <esr@thyrsus.com>
> Cc: emacs-devel@gnu.org
> 
> So I reverted to the trunk version and instrumented, by inserting an
> echo before the generated make command.  Lo and behold, vcswitness was
> empty.

Is it still empty for you with the current trunk?  (It is not empty
for me.)  If so, do you have a .bzr/checkout/dirstate file under the
top-level directory of your bzr branch?  Also, did you try that in the
bzr branch, or in the directory where you build (assuming that they
are different, i.e. that you don't build in-place)?

> Those three lines are, by demonstration, so prone to be misread that they 
> tripped up Eli and me in two different ways and I still have no theory
> to account for why they were different.
> 
> That says "maintainence nightmare - should be scrapped and replaced"
> to me.

Maybe so, but replaced with what?

The main requirement for this trick is that the value of VCSWITNESS is
computed in the top-level Makefile, but used in a sub-Make.  So the
variables that should be expanded by the sub-Make should be quoted,
while those that need to be expanded by the top-level Make should not
be quoted.  Is there a better way of expressing that than what we have
now?

Re: VCSWITNESS = fail ** 2

[Glenn Morris]

These changes should NOT be happening during a feature freeze.
Please stop it.
Please revert this code to how it was prior to r115992.

If you think there is something wrong with it, make a CLEAR bug report,
and wait for people to comment.

The feature freeze is likely to end in no more than a few weeks.
There is zero rush to make any of these Git changes.

Re: VCSWITNESS = fail ** 2

[David Kastrup]

"Eric S. Raymond" <esr@thyrsus.com> writes:

> Eli Zaretskii <eliz@gnu.org>:
>> Actually, I take that back: the original code should work, and works
>> for me.
>> 
>> So let's please step back one notch: can you please tell what exactly
>> is wrong with the way vcswitness is computed and passed to the
>> sub-Make?
>
> What I know is this: 
>
> I was trying to modify that code to DTRT in both a Bazaar and .git repo, 
> using .git/HEAD as a witness in the latter case.  The way my modification 
> failed caused me to suspect that the original code hadn't been working.
>
> So I reverted to the trunk version and instrumented, by inserting an
> echo before the generated make command.  Lo and behold, vcswitness was
> empty.
>
> I looked again.  Had I doubled $ properly for the make context?  I had;
> vcswitness was really empty.  (But I was already pretty sure of that; if
> doubling $$ had been incorrect it would have expanded as somebody's 
> process number.)

More likely than not, you missed one \ somewhere, resulting in a
_different_ shell being called which then never heard of vcswitness.

> So I posted to the list and waited for somebody to point out the flaw
> in my reasoning.  Nobody did. Meanwhile, in my own test builds
> vcswitness continued to be spuriously empty.
>
> The rest you all know.  Eli took his own comic pratfall, momentarily
> breaking the build. And he's been around the track a few times
> himself.
>
> At present, I have no explanation for the observed differences in
> behavior more predictive than "phase of the moon".

Please read the previous two paragraphs together.  You ridicule Eli for
not managing to fix your breakage in the first try, and then state that
you yourself have no idea what caused it.

Now we may have clashing/incompatible personalities: that's to be
expected.  But please note that you are _not_ measuring yourself with
the same standards you apply to others.  If we had two Eric Raymonds
involved here, one would mercilessly mock the other one.

This is not a problem of the mailing list: it is a problem of yourself.

> Those three lines are, by demonstration, so prone to be misread that
> they tripped up Eli and me in two different ways and I still have no
> theory to account for why they were different.

And yet you spout things like "Eli took his own comic pratfall,
momentarily breaking the build".

It's not like Eli and I get along with each other.  But you are clearly
completely out of line here.

> That says "maintainence nightmare - should be scrapped and replaced"
> to me.

You did not offer any replacement or a suitable suggestion for
replacement.  Yes, your own comic pratfall suggests that it's not
trivial to understand.  But nobody claimed that.

-- 
David Kastrup

Re: VCSWITNESS = fail ** 2

[Eric S. Raymond]

David Kastrup <dak@gnu.org>:
> Please read the previous two paragraphs together.  You ridicule Eli for
> not managing to fix your breakage in the first try, and then state that
> you yourself have no idea what caused it.

!!! I wasn't ridiculing Eli, I was implicitly poking fun at myself.  The
antecedent of "his own comic pratfall" is that *I* took one.

As for the rest of your response, I'll pass over it in polite silence.
But your chances of successfully influencing my future behavior just
decreased.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

Re: VCSWITNESS = fail ** 2

[David Kastrup]

"Eric S. Raymond" <esr@thyrsus.com> writes:

> David Kastrup <dak@gnu.org>:
>> Please read the previous two paragraphs together.  You ridicule Eli for
>> not managing to fix your breakage in the first try, and then state that
>> you yourself have no idea what caused it.
>
> !!! I wasn't ridiculing Eli, I was implicitly poking fun at myself.

You take yourself more important than others if you consider
_implicitly_ poking fun at yourself an adequate justification for
_explicitly_ poking fun at somebody else.  In particular if you keep the
former to yourself.

> As for the rest of your response, I'll pass over it in polite silence.
> But your chances of successfully influencing my future behavior just
> decreased.

I did not consider them high to start with, but would have considered it
inappropriate to leave Eli to deal on his own with what amounts to
friendly, self-deprecating bullying.  The choice of communication
standards involves two parties.

-- 
David Kastrup

Re: VCSWITNESS = fail ** 2

[Eli Zaretskii]

> From: Glenn Morris <rgm@gnu.org>
> Date: Sun, 12 Jan 2014 14:10:22 -0500
> 
> 
> These changes should NOT be happening during a feature freeze.
> Please stop it.
> Please revert this code to how it was prior to r115992.

It is already that way.  Try

  bzr diff -r 115991.. Makefile.in

> There is zero rush to make any of these Git changes.

Indeed, I suggest to do all of that on a branch.

Re: VCSWITNESS = fail ** 2

[Eli Zaretskii]

> Date: Sun, 12 Jan 2014 14:42:23 -0500
> From: "Eric S. Raymond" <esr@thyrsus.com>
> Cc: emacs-devel@gnu.org
> 
> David Kastrup <dak@gnu.org>:
> > Please read the previous two paragraphs together.  You ridicule Eli for
> > not managing to fix your breakage in the first try, and then state that
> > you yourself have no idea what caused it.
> 
> !!! I wasn't ridiculing Eli, I was implicitly poking fun at myself.  The
> antecedent of "his own comic pratfall" is that *I* took one.
> 
> As for the rest of your response, I'll pass over it in polite silence.
> But your chances of successfully influencing my future behavior just
> decreased.

Guys?  Can we please all cool down here and put this incident behind
us?  Anyone could make a mistake (even me ;-).

I'd still want to know what's wrong with the original code, because I
cannot see anything wrong with it.

Re: VCSWITNESS = fail ** 2

[Glenn Morris]

Eli Zaretskii wrote:

> It is already that way.

Yes, thanks, I was confused by the multiple partial reverts.

>> There is zero rush to make any of these Git changes.
>
> Indeed, I suggest to do all of that on a branch.

I already suggested that too.

Re: VCSWITNESS = fail ** 2

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

    Please always wait for at least a day, if not longer, before you
    conclude that no one responded.

It is better to wait 2 days.  What's the rush?

-- 
Dr Richard Stallman
President, Free Software Foundation
51 Franklin St
Boston MA 02110
USA
www.fsf.org  www.gnu.org
Skype: No way! That's nonfree (freedom-denying) software.
  Use Ekiga or an ordinary phone call.