emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

VCSWITNESS = fail ** 2


From: Eric S. Raymond
Subject: VCSWITNESS = fail ** 2
Date: Sun, 12 Jan 2014 09:37:33 -0500 (EST)

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]