Mopre fun with vcswitness

Mopre fun with vcswitness

[Eric S. Raymond]

This as meant to be a reply to Eli's last, but I dropped that mail somehow.

With current trunk (115999) and one line of instrumebtation added,
this is what I see doing an in-place build:

dirstate='.bzr/checkout/dirstate';				\
	vcswitness='$(srcdir)/../'$dirstate;				\
	[ -r "./$dirstate" ] || vcswitness='';			\
	cd src || exit;							\
	boot=bootstrap-emacs;					\
	[ ! -x "$boot" ] || boot='';					\
	echo "VCSWITNESS: $vcswitness";\
	make all 						\
	  CC='gcc -std=gnu99' CFLAGS='-g3 -O2' CPPFLAGS=''		\
	  LDFLAGS='' MAKE='make' BOOTSTRAPEMACS="$boot"	\
	  VCSWITNESS="$vcswitness"
VCSWITNESS: $(srcdir)/../.bzr/checkout/dirstate

While this is nonempty, it's not going to work either.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

"The state calls its own violence `law', but that of the individual `crime'"
	-- Max Stirner

Re: Mopre fun with vcswitness

[Andreas Schwab]

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

> VCSWITNESS: $(srcdir)/../.bzr/checkout/dirstate
>
> While this is nonempty, it's not going to work either.

It will work once you feed it through make.  Look up `recursively
expanded variables' in the make manual.

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: Mopre fun with vcswitness

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Date: Sun, 12 Jan 2014 21:14:44 +0100
> Cc: emacs-devel@gnu.org
> 
> esr@thyrsus.com (Eric S. Raymond) writes:
> 
> > VCSWITNESS: $(srcdir)/../.bzr/checkout/dirstate
> >
> > While this is nonempty, it's not going to work either.
> 
> It will work once you feed it through make.

Indeed.  srcdir is expanded by Make when it processes src/Makefile,
which is the reason for the ".." part.

Eric, you need to show the value inside src/Makefile, where it is used
as a dependency.

Re: Mopre fun with vcswitness

[David Kastrup]

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

> This as meant to be a reply to Eli's last, but I dropped that mail somehow.
>
> With current trunk (115999) and one line of instrumebtation added,
> this is what I see doing an in-place build:
>
> dirstate='.bzr/checkout/dirstate';				\
> 	vcswitness='$(srcdir)/../'$dirstate;				\
> 	[ -r "./$dirstate" ] || vcswitness='';			\
> 	cd src || exit;							\
> 	boot=bootstrap-emacs;					\
> 	[ ! -x "$boot" ] || boot='';					\
> 	echo "VCSWITNESS: $vcswitness";\
> 	make all 						\
> 	  CC='gcc -std=gnu99' CFLAGS='-g3 -O2' CPPFLAGS=''		\
> 	  LDFLAGS='' MAKE='make' BOOTSTRAPEMACS="$boot"	\
> 	  VCSWITNESS="$vcswitness"
> VCSWITNESS: $(srcdir)/../.bzr/checkout/dirstate
>
> While this is nonempty, it's not going to work either.

I don't see that.  For an in-place build, srcdir will be '.', so after
cd src, VCSWITNESS will be correct.

The overall look is highly suspicious for srcdir being different from
. but for . itself it would seem like it would work.

No idea whether it is by accident more than by design: that would have
to be seen by taking a look at the results for a non-in-place build.

-- 
David Kastrup

Re: Mopre fun with vcswitness

[Eric S. Raymond]

David Kastrup <dak@gnu.org>:
> > VCSWITNESS: $(srcdir)/../.bzr/checkout/dirstate
> >
> > While this is nonempty, it's not going to work either.
> 
> I don't see that.

I believe you.  Which just increases my distrust of this code.

You and I and Eli are running in environments we think ought not to be
significantly different (in-place builds) and yet we get at least two
and possibly three different behaviors (Eli hasn't reported his
exactly enough for me to be sure).

To add to the fun, I think this is different behavior than I was
previously seeing from putatively identical code.  There's got to be
some hidden state we're all missing.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

Re: Mopre fun with vcswitness

[David Kastrup]

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

> David Kastrup <dak@gnu.org>:
>> > VCSWITNESS: $(srcdir)/../.bzr/checkout/dirstate
>> >
>> > While this is nonempty, it's not going to work either.
>> 
>> I don't see that.
>
> I believe you.  Which just increases my distrust of this code.
>
> You and I and Eli are running in environments we think ought not to be
> significantly different (in-place builds) and yet we get at least two
> and possibly three different behaviors (Eli hasn't reported his
> exactly enough for me to be sure).

"I don't see that" does not mean that I see a particular behavior when
running the code (I did not do so), but it rather means that I don't
agree with your conclusion.

> To add to the fun, I think this is different behavior than I was
> previously seeing from putatively identical code.

Where "putatively" means what _you_ remember being identical.  I already
opined that I consider it most likely you forgot a \ in your added code,
leading to different shell instances defining and printing vcswitness.
Quite a common Makefile fallacy.  An ugly design decision in my opinion,
but then it's been around for more than 30 years I think, so it's not
really likely to change.

> There's got to be some hidden state we're all missing.

Because you cannot reproduce what you think you saw before?  That's a
mystery nobody but yourself has a chance of addressing.

-- 
David Kastrup

Re: Mopre fun with vcswitness

[Eric S. Raymond]

David Kastrup <dak@gnu.org>:
> Where "putatively" means what _you_ remember being identical.  I already
> opined that I consider it most likely you forgot a \ in your added code,
> leading to different shell instances defining and printing vcswitness.

No, whatever other mistakes I may have made, that wasn't one of them.  The 
aligned column of backslashes off to the right is helpful that way. I've
been *very* aware of them the whole time.

It would probably be a good idea for somebody tp put a permanent line
of instrumentation in there so we all know we're seeing the sane report. 
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

Re: Mopre fun with vcswitness

[Eli Zaretskii]

> From: David Kastrup <dak@gnu.org>
> Date: Sun, 12 Jan 2014 21:18:20 +0100
> 
> The overall look is highly suspicious for srcdir being different
> from .

srcdir should always be a relative file name, like ../foo, never
absolute.

Does it still look suspicious under that assumption?

Re: Mopre fun with vcswitness

[David Kastrup]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: David Kastrup <dak@gnu.org>
>> Date: Sun, 12 Jan 2014 21:18:20 +0100
>> 
>> The overall look is highly suspicious for srcdir being different
>> from .
>
> srcdir should always be a relative file name, like ../foo, never
> absolute.
>
> Does it still look suspicious under that assumption?

Yes.  I am not saying "guilty", I am saying "suspicious".  While the
U.S. justice system makes it very expensive to get differentiation
between "guilty" and "suspicious", it's still a valid distinction.

But it looks filthy enough to be viewed as within three hops of a
genuine bug.

At any rate, even with relative paths I'd not expect to see

    VCSWITNESS: $(srcdir)/../.bzr/checkout/dirstate

but rather

    VCSWITNESS: ../$(srcdir)/.bzr/checkout/dirstate

since it is hard to imagine that the former would work for _anything_
but srcdir=. here.

The same holds for the current

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

which would seem, used from within the src subdirectory, to have to
rather be

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

It would seem that srcdir=. is the _only_ value for which this would
work.  Which is also likely the most common value, namely the one for
in-place builds.

-- 
David Kastrup

Re: More fun with vcswitness

[Jan Djärv]

Hello.

12 jan 2014 kl. 22:12 skrev Eli Zaretskii <eliz@gnu.org>:

> srcdir should always be a relative file name, like ../foo, never
> absolute.

Is this the srcdir used when compiling in a separate directory?
Then there is a problem.  See bug 15795 (http://debbugs.gnu.org/cgi/bugreport.cgi?bug=15795#11) where this was said:

4 nov 2013 kl. 09:10 skrev Glenn Morris <rgm@gnu.org>:

> 
> PS try configuring your out-of-tree build using an absolute path to the
> srcdir.

So srcdir is recommended to be absolute in this case.  Not that I've seen anywhere stating that separate build directory should use relative paths.
Of course if compile mode was smarter this could go away.

Ignore if you are talking about another srcdir.

	Jan D.

Re: More fun with vcswitness

[Glenn Morris]

Jan Djärv wrote:

> 12 jan 2014 kl. 22:12 skrev Eli Zaretskii <eliz@gnu.org>:
>
>> srcdir should always be a relative file name, like ../foo, never
>> absolute.
>
> Is this the srcdir used when compiling in a separate directory?

Yes, you are right, it may or may not be absolute.

Personally I don't see a problem, since '$srcdir' is single quoted, so
that it gets expanded only in src/Makefile, as Andreas said.

But there are too many emails of supposition, and AFAICS still no clear
bug report, so I dunno.

And none of this affects Emacs releases anyway, so it has zero relevance
to 24.4 and is not worth spending time on now.

Re: More fun with vcswitness

[Eli Zaretskii]

> From: Glenn Morris <rgm@gnu.org>
> Cc: Eli Zaretskii <eliz@gnu.org>,  emacs-devel <emacs-devel@gnu.org>
> Date: Sun, 12 Jan 2014 21:46:41 -0500
> 
> Jan Djärv wrote:
> 
> > 12 jan 2014 kl. 22:12 skrev Eli Zaretskii <eliz@gnu.org>:
> >
> >> srcdir should always be a relative file name, like ../foo, never
> >> absolute.
> >
> > Is this the srcdir used when compiling in a separate directory?
> 
> Yes, you are right, it may or may not be absolute.
> 
> Personally I don't see a problem, since '$srcdir' is single quoted, so
> that it gets expanded only in src/Makefile, as Andreas said.

Perhaps using abs_srcdir will allow to use the same string everywhere,
avoiding the need to sometimes use ".." and sometimes quote.

Re: More fun with vcswitness

[Stefan Monnier]

> Perhaps using abs_srcdir will allow to use the same string everywhere,
> avoiding the need to sometimes use ".." and sometimes quote.

In any case "$(srcdir)/../.bzr" doesn't make as much sense as
"../$(srcdir)/.bzr".


        Stefan

Re: More fun with vcswitness

[Andreas Schwab]

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

> In any case "$(srcdir)/../.bzr" doesn't make as much sense as
> "../$(srcdir)/.bzr".

??? The latter makes absolutely no sense, especially if $(srcdir) is an
absolute name.  What you really want is $(top_srcdir).

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: More fun with vcswitness

[David Kastrup]

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

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>> In any case "$(srcdir)/../.bzr" doesn't make as much sense as
>> "../$(srcdir)/.bzr".
>
> ??? The latter makes absolutely no sense, especially if $(srcdir) is an
> absolute name.  What you really want is $(top_srcdir).

So srcdir is different in every directory?

-- 
David Kastrup

Re: More fun with vcswitness

[Andreas Schwab]

David Kastrup <dak@gnu.org> writes:

> Andreas Schwab <schwab@linux-m68k.org> writes:
>
>> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>
>>> In any case "$(srcdir)/../.bzr" doesn't make as much sense as
>>> "../$(srcdir)/.bzr".
>>
>> ??? The latter makes absolutely no sense, especially if $(srcdir) is an
>> absolute name.  What you really want is $(top_srcdir).
>
> So srcdir is different in every directory?

It points to the directory containing the sources being compiled.

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: More fun with vcswitness

[Stefan Monnier]

>> In any case "$(srcdir)/../.bzr" doesn't make as much sense as
>> "../$(srcdir)/.bzr".
> ??? The latter makes absolutely no sense, especially if $(srcdir) is an
> absolute name.  What you really want is $(top_srcdir).

Right, I was thinking of $(srcdir) as the root of the checkout:
../$(srcdir)/.bzr wouldn't make sense for an absolute (srcdir),
but $(srcdir)/../.bzr makes even less sense if $(srcdir) as the root of
the checkout.


        Stefan

Re: More fun with vcswitness

[Glenn Morris]

Stefan Monnier wrote:

> Right, I was thinking of $(srcdir) as the root of the checkout:
> ../$(srcdir)/.bzr wouldn't make sense for an absolute (srcdir),
> but $(srcdir)/../.bzr makes even less sense if $(srcdir) as the root of
> the checkout.

srcdir is not the root of the checkout. top_srcdir is.
srcdir is "directory containing the sources associated with the Makefile
that expands it". In many cases it will be "." everywhere: in Makefile,
src/Makefile, etc. In another case it might be ../emacs in Makefile,
../../emacs/src in src/Makefile, etc.

$(top_srcdir)/.bzr is equivalent to what you have now, but AFAICS
there's no _need_ to change it, except maybe clarity.

Re: More fun with vcswitness

[Glenn Morris]

Eli Zaretskii wrote:

> Perhaps using abs_srcdir will allow to use the same string everywhere,
> avoiding the need to sometimes use ".." and sometimes quote.

abs_srcdir may contain whitespace, which is a problem for Make.

(srcdir is guaranteed not to, by configure.)

The current code seems fine to me.

Re: More fun with vcswitness

[Eli Zaretskii]

> From: Glenn Morris <rgm@gnu.org>
> Date: Mon, 13 Jan 2014 14:03:13 -0500
> Cc: jan.h.d@swipnet.se, emacs-devel@gnu.org
> 
> The current code seems fine to me.

Me too.  It's just that its use of quotes of different kinds might
confuse.