Bug 260128 - "make tags" fails with "gtags not found", src/Makefile undesirably rewrites PATH
Summary: "make tags" fails with "gtags not found", src/Makefile undesirably rewrites PATH
Status: New
Alias: None
Product: Base System
Classification: Unclassified
Component: misc (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-11-30 00:34 UTC by Jory Folker
Modified: 2021-12-04 22:56 UTC (History)
3 users (show)

See Also:


Attachments
proposed patch (764 bytes, patch)
2021-12-04 22:47 UTC, Jory Folker
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jory Folker 2021-11-30 00:34:14 UTC
When running "make tags" to generate a TAGS table for the src tree, I encountered the build failure shown at the bottom of this bug report. When I put a symlink to gtags in /usr/bin/, "make tags" succeeds.

It seems that somewhere in Makefile Land, a sweaty fever dream of a place where clean targets either clean only half your kitchen or burn it to the ground, PATH isn't just being prepended or appended, but flat-out erased and rewritten.

Sadly, the cause is not obvious from a cursory recursion into every Makefile from src/Makefile all the way down to src/lib/csu/tests/dso/Mekefile where the build fails.

I'll keep digging until I find the skullduggery!

root@freebsd:/home/jfolker/freebsd/src # echo $PATH
/sbin:/bin:/usr/sbin:/usr/bin:/usr/local/sbin:/usr/local/bin:/root/bin
root@freebsd:/home/jfolker/freebsd/src # which gtags
/usr/local/bin/gtags
root@freebsd:/home/jfolker/freebsd/src # pwd
/usr/home/jfolker/freebsd/src
root@freebsd:/home/jfolker/freebsd/src # make tags
===> lib (tags)
===> lib/csu (tags)
===> lib/csu/amd64 (tags)
===> lib/csu/tests (tags)
===> lib/csu/tests/dso (tags)
/bin/sh: gtags: not found
*** Error code 127

Stop.
make[5]: stopped in /usr/home/jfolker/freebsd/src/lib/csu/tests/dso
*** Error code 1

Stop.
make[4]: stopped in /usr/home/jfolker/freebsd/src/lib/csu/tests
*** Error code 1

Stop.
make[3]: stopped in /usr/home/jfolker/freebsd/src/lib/csu
*** Error code 1

Stop.
make[2]: stopped in /usr/home/jfolker/freebsd/src/lib
*** Error code 1

Stop.
make[1]: stopped in /usr/home/jfolker/freebsd/src
*** Error code 1

Stop.
make: stopped in /usr/home/jfolker/freebsd/src
root@freebsd:/home/jfolker/freebsd/src #
Comment 1 Jory Folker 2021-11-30 03:24:32 UTC
As it turns out, I was too groggy from coughing my lungs out (and NyQuil) to notice the cause sitting right there in plain sight, line 215 of Makefile:

PATH= /sbin:/bin:/usr/sbin:/usr/bin

This is flat-out wrong. Note that the first n directories in the default PATH for root are identical, in line 22 of /root/.cshrc:

set path = (/sbin /bin /usr/sbin /usr/bin /usr/local/sbin /usr/local/bin $HOME/bin)

If the user prepends directories onto PATH in /root/.cshrc and breaks their own build by calling a version of an executable (e.g. gtags) not explicitly maintained for build purposes, I don't think it's unreasonable to call it pilot error.

We should also consider the undue hassle to some LLVM maintainer testing their experimental clang build.

My proposed fix to remove this line from src/Makefile is included at the bottom of this message.


215d214
< PATH= /sbin:/bin:/usr/sbin:/usr/bin
Comment 2 Jory Folker 2021-11-30 03:41:04 UTC
... Lest I forget to use "diff -u"...

@@ -212,7 +212,6 @@
 .ORDER: buildkernel reinstallkernel
 .ORDER: buildkernel reinstallkernel.debug
 
-PATH=	/sbin:/bin:/usr/sbin:/usr/bin
 MAKEOBJDIRPREFIX?=	/usr/obj
 _MAKEOBJDIRPREFIX!= /usr/bin/env -i PATH=${PATH} ${MAKE} MK_AUTO_OBJ=no \
     ${.MAKEFLAGS:MMAKEOBJDIRPREFIX=*} __MAKE_CONF=${__MAKE_CONF} \
Comment 3 Jessica Clarke freebsd_committer 2021-11-30 14:55:59 UTC
That's not the right fix. The build is deliberately done with a hard-coded path to ensure nothing outside of the base system is used accidentally. If a target needs something from /usr/local/bin then it should either default to using an absolute path or add to PATH temporarily and locally when needed. That is, CTAGS in bsd.dep.mk should likely be defaulting to /usr/local/bin/gtags (or, really, ${LOCALBASE}/bin/gtags), though maybe base's ctags works well enough (the default was flipped in 2002 from ctags to gtags, and I don't know what the status of either of them is these days).
Comment 4 Jory Folker 2021-11-30 22:55:35 UTC
For now, I propose setting /usr/local/bin/gtags in bsd.dep.mk, and here is my diff:

freebsd% cat bsd.dep.mk.diff
--- bsd.dep.mk	2021-11-30 16:09:01.231877000 -0600
+++ bsd.dep.mk.new	2021-11-30 16:08:25.988490000 -0600
@@ -45,7 +45,7 @@
 .error bsd.dep.mk cannot be included directly.
 .endif
 
-CTAGS?=		gtags
+CTAGS?=		/usr/local/bin/gtags
 CTAGSFLAGS?=
 GTAGSFLAGS?=	-o
 HTAGSFLAGS?=
freebsd% 

If we could eliminate a dependency on an executable from outside the base system, then defaulting to ctags and allowing the user to change it by exporting CTAGS in their env is ideal.

However, when I run 'make tags' with ctags, stderr is flooded with superfluous "Duplicate entry" warnings for redefined macros, functions defined using macros, etc, like this:

Duplicate entry in file /usr/home/jfolker/freebsd/src/contrib/ntp/ntpdc/ntpdc.c, line 1910: __attribute__
Second entry ignored
Duplicate entry in file /usr/home/jfolker/freebsd/src/contrib/ntp/ntpdc/ntpdc.c, line 1926: __attribute__
Second entry ignored

This isn't itself bad, but it gives reasonable suspicion that we will lose other functionality switching from gtags to ctags.

But I found something else worth noting: unless the install dest for GNU global was changed in the ports tree Makefile, 'make tags' has been broken for over 18 years.

freebsd% git blame src/Makefile
d672a60908a4 (Ruslan Ermilov      2002-04-29 15:22:01 +0000 214) 
76499f153979 (Ruslan Ermilov      2003-01-29 10:00:42 +0000 215) PATH=  /sbin:/bin:/usr/sbin:/usr/bin
2e84ab947d9c (Ruslan Ermilov      2002-12-02 14:31:21 +0000 216) MAKEOBJDIRPREFIX?=     /usr/obj
freebsd%
freebsd% git blame src/share/mk/bsd.dep.mk
5ffdf3618ef7 (Ruslan Ermilov      2002-04-22 10:04:41 +0000  44) .if !target(__<bsd.init.mk>__)
5ffdf3618ef7 (Ruslan Ermilov      2002-04-22 10:04:41 +0000  45) .error bsd.dep.mk cannot be included directly.
5ffdf3618ef7 (Ruslan Ermilov      2002-04-22 10:04:41 +0000  46) .endif
b16495e786c9 (Wolfram Schneider   1996-04-01 18:58:28 +0000  47) 
ccc4bab1047d (Ruslan Ermilov      2002-10-17 13:48:13 +0000  48) CTAGS?=                gtags
ccc4bab1047d (Ruslan Ermilov      2002-10-17 13:48:13 +0000  49) CTAGSFLAGS?=
ccc4bab1047d (Ruslan Ermilov      2002-10-17 13:48:13 +0000  50) GTAGSFLAGS?=   -o
ccc4bab1047d (Ruslan Ermilov      2002-10-17 13:48:13 +0000  51) HTAGSFLAGS?=
ccc4bab1047d (Ruslan Ermilov      2002-10-17 13:48:13 +0000  52)
Comment 5 Warner Losh freebsd_committer 2021-12-03 23:23:08 UTC
(In reply to Jory Folker from comment #4)
> +CTAGS?=		/usr/local/bin/gtags

I think this is good. I'd spell it "${LOCALBASE}/bin/gtags"

As has been noted elsewhere, ru's changes were on purpose to ensure that we had sequestered binaries for certain critical parts of the build process.
Comment 6 Jory Folker 2021-12-04 22:47:54 UTC
Created attachment 229907 [details]
proposed patch
Comment 7 Jory Folker 2021-12-04 22:56:05 UTC
Alas! LOCALBASE is not set in bsd.dep.mk anywhere up the Makefile call chain for 'make tags'.

Thus, I attached a proposed patch that sets 'LOCALBASE?= /usr/local' in the top-level Makefile, and one more time in bsd.dep.mk because:

(a) I see this line in 4 other .mk files and 9 other Makefiles
2. Belt and suspenders.

I maintain my protest that hardcoding PATH in a Makefile is not kosher for reasons outlined in comment #1 with extra emphasis on "DO NOT MESS WITH /root/.cshrc", but since leaving it alone has no practical implications, I'm content to make like Paul McCartney and let it be.