Bug 202277
| Summary: | [patch] src/Makefile.inc1 - fix buildworld from 9.3-stable to 10.2-stable (Malformed conditional after r285814) | ||||||
|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | John Hein <jcfyecrayz> | ||||
| Component: | misc | Assignee: | Glen Barber <gjb> | ||||
| Status: | Closed FIXED | ||||||
| Severity: | Affects Some People | CC: | jcfyecrayz, re | ||||
| Priority: | --- | Keywords: | patch | ||||
| Version: | 10.2-STABLE | Flags: | jcfyecrayz:
mfc-stable10?
|
||||
| Hardware: | Any | ||||||
| OS: | Any | ||||||
| Attachments: |
|
||||||
Looks like this affects 10.2-release, too. CC'ing re@ - seems like it may be of interest for upcoming 10.2 release. We'll have to issue an EN for this after 10.2-RELEASE is out. The release builds are already in progress. Investigating further, it appears this is a non-fatal issue when upgrading, and produces (still unfortunate) noise during invocation. The patch you propose is already in head, but was never MFC'd to stable/10. We will pursue an EN for this after the 10.2-RELEASE is announced. Thank you for the report. Thanks for looking into it. Yes, I see it was fixed in r262670 on head back in March 2014. I agree it's non-fatal (for the build) - I should have mentioned that in the original report (sorry). But I didn't track down the ramifications of a VERSION string that is missing the REVISION & BRANCH elements. Or rather I briefly saw that VERSION is passed in the environment to some sub-makes, but I didn't find any places where it was referenced beyond that (quick grep of Makefile* & *.mk in the tree didn't show anything that would use the Makefile.inc1 VERSION string, but I may have missed it). (In reply to John Hein from comment #5) > Thanks for looking into it. No problem. Thank you for reporting the issue. > Yes, I see it was fixed in r262670 on head back in March 2014. > > I agree it's non-fatal (for the build) - I should have mentioned that in the > original report (sorry). > That's fine. > But I didn't track down the ramifications of a VERSION string that is > missing the REVISION & BRANCH elements. Or rather I briefly saw that > VERSION is passed in the environment to some sub-makes, but I didn't find > any places where it was referenced beyond that (quick grep of Makefile* & > *.mk in the tree didn't show anything that would use the Makefile.inc1 > VERSION string, but I may have missed it). It has zero rammifications for the world and kernel build that I was able to determine. As far as I am able to tell, it is only used for setting the OSRELEASE for the actual release build, when the installation images are renamed from 'disc1.iso' to 'FreeBSD-10.2-RELEASE-amd64-disc1.iso', for example. Okay. It looks like release/Makefile sets OSRELEASE from newvers.sh rather than any VERSION string in the environment. Maybe there's somewhere VERSION is used, but I can't find it right now [1]. If not, maybe all that stuff (REVISION, BRANCH, VERSION) can be removed. I'll see if I can find out where it came from with some archaeology. [1] release/scripts/atlas-upload.sh could be slightly confused by VERSION in the environment. It should probably unset its internal variables early. VERSION was added in 2008 by jb@ (r179233). So VERSION is encoded in ctf sections in the kernel somehow. I can't find where that is implemented right now, but commit logs for Makefile.inc1 talk about it. I see it now... sys.mk adds '-L VERSION' to CTFFLAGS which tells ctfconvert to read the label from the environment variable VERSION.
So the upshot of the missing bits of VERSION is that if you're building with CTF and updating from 9 to 10, a somewhat unexpected label will live in the ctf section of object files, missing REVISION & BRANCH. If your kernel & world object files are all built that way, everything would be consistent. But it's not clear to me how these malformed ctf labels would make a big difference when doing dtrace things anyway. All in all, it seems like it won't hurt anything seriously. If that's so, I don't know if I would bother with an errata notice. That said, it'd be nice if someone could merge the ${MAKE} change from head to 10/stable - if nothing else, just to avoid the extra noise in the build.
(In reply to John Hein from comment #10) > I see it now... sys.mk adds '-L VERSION' to CTFFLAGS which tells ctfconvert > to read the label from the environment variable VERSION. > > So the upshot of the missing bits of VERSION is that if you're building with > CTF and updating from 9 to 10, a somewhat unexpected label will live in the > ctf section of object files, missing REVISION & BRANCH. If your kernel & > world object files are all built that way, everything would be consistent. > But it's not clear to me how these malformed ctf labels would make a big > difference when doing dtrace things anyway. All in all, it seems like it > won't hurt anything seriously. If that's so, I don't know if I would bother > with an errata notice. That said, it'd be nice if someone could merge the > ${MAKE} change from head to 10/stable - if nothing else, just to avoid the > extra noise in the build. The change for stable/10 will be made shortly. The EN for 10.2-RELEASE will be for the 's/make/${MAKE}/' change, so the noisy output is suppressed in an updated releng/10.2 upgrade from 9.x and earlier. A commit references this bug: Author: gjb Date: Thu Aug 13 22:29:27 UTC 2015 New revision: 286746 URL: https://svnweb.freebsd.org/changeset/base/286746 Log: MFC r262670 (marcel): Use ${MAKE} so that we always use the same version/implementation of make(1). PR: 202277 Submitted by: John Hein Sponsored by: The FreeBSD Foundation Changes: _U stable/10/ stable/10/Makefile.inc1 Fixed in 10.2-RELEASE as FreeBSD-EN-15:11.toolchain. Thank you, again, for the report. |
Created attachment 159819 [details] use ${MAKE} in Makefile.inc1 for setting REVISION & BRANCH When updating from 9.3-stable to 10.2-stable, you get: make buildworld . . "/usr/src/release/Makefile.vagrant", line 27: Malformed conditional () "/usr/src/release/Makefile.vagrant", line 27: Malformed conditional () "/usr/src/release/Makefile.vagrant", line 29: if-less endif "/usr/src/release/Makefile.vagrant", line 27: Malformed conditional () "/usr/src/release/Makefile.vagrant", line 29: if-less endif "/usr/src/release/Makefile.vagrant", line 27: Malformed conditional () "/usr/src/release/Makefile.vagrant", line 29: if-less endif "/usr/src/release/Makefile.vagrant", line 31: if-less endif "/usr/src/release/Makefile.vagrant", line 64: Malformed conditional (azure == "virtualbox") "/usr/src/release/Makefile.vagrant", line 67: if-less elif "/usr/src/release/Makefile.vagrant", line 70: if-less endif "/usr/src/release/Makefile.vagrant", line 74: Malformed conditional (azure == "virtualbox") "/usr/src/release/Makefile.vagrant", line 76: if-less elif "/usr/src/release/Makefile.vagrant", line 78: if-less endif "/usr/src/release/Makefile.vagrant", line 64: Malformed conditional (ec2 == "virtualbox") "/usr/src/release/Makefile.vagrant", line 67: if-less elif . . This is caused by these lines from Makefile.inc1 (at least r286673 and earlier): REVISION!= make -C ${SRCDIR}/release -V REVISION BRANCH!= make -C ${SRCDIR}/release -V BRANCH This uses fmake (9.3-stable's /usr/bin/make) and release/Makefile.vagrant (included by release/Makefile.vm which is included by release/Makefile) has some bmake-isms in it (as of July 23's r285814 on 10-stable). Makefile.vagrant could be scrubbed of bmake-isms, but it's just as easy and probably better to use ${MAKE} instead of 'make' in Makefile.inc1. The patch I'll attach uses that approach (can commit to HEAD and MFC to 10-stable).