Bug 215401 - Mk/bsd.port.mk: don't clobber ARCH value for do-build
Summary: Mk/bsd.port.mk: don't clobber ARCH value for do-build
Status: Open
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Ports Framework (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Port Management Team
Keywords: patch
Depends on:
Blocks: 217199
  Show dependency treegraph
Reported: 2016-12-19 01:40 UTC by Jan Beich
Modified: 2018-05-23 10:27 UTC (History)
5 users (show)

See Also:
jbeich: exp-run?

v0 (15.54 KB, patch)
2016-12-19 01:40 UTC, Jan Beich
jbeich: maintainer-approval? (bdrewery)
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Beich freebsd_committer 2016-12-19 01:40:58 UTC
Created attachment 178082 [details]

ports r20327 added an unsafe optimization for caching ARCH value. Rather than run uname(1) let's use MACHINE_ARCH instead as both query sysctl hw.machine_arch, at least in bmake.
Comment 1 Mark Linimon freebsd_committer freebsd_triage 2016-12-19 05:32:50 UTC
I guess I'm confused.  I don't see the annotation for r20327.  Is this possibly a typo?
Comment 2 Jan Beich freebsd_committer 2016-12-19 06:32:04 UTC
Just a bug in our bugzilla configuration. Here again: ports r20327
Comment 3 Antoine Brodin freebsd_committer 2016-12-19 17:43:04 UTC
When cross building,  poudriere sets ARCH=${MACHINE_ARCH} in make.conf

Does this have an impact with your patch?  I guess not yet?
Comment 4 Jan Beich freebsd_committer 2016-12-19 18:32:43 UTC
Variables in make.conf don't clobber values in vender Makefile(s) unlike .MAKEFLAGS or those passed on command-line. The exp-run here is to catch unknown dependencies on such a behavior. |make index| maybe a bit faster but not beyond ~ 1% if not at all lost in the noise.

> When cross building,  poudriere sets ARCH=${MACHINE_ARCH} in make.conf

It's going to be the default for everyone.
Comment 5 Jan Beich freebsd_committer 2016-12-19 18:56:42 UTC
Comment on attachment 178082 [details]

Adding bdrewery for review to avoid rubberstamping after exp-run. If you're curious he did promise to fix the issue... a year ago.

Comment 6 Antoine Brodin freebsd_committer 2016-12-22 07:11:35 UTC
Take for exp-run.  Review is still welcome.
Comment 7 Antoine Brodin freebsd_committer 2016-12-25 08:36:03 UTC
Exp-run is fine on 11.0 amd64 and 11.0 i386
Comment 8 Jan Beich freebsd_committer 2016-12-25 09:09:31 UTC
(In reply to Antoine Brodin from comment #7)
> Exp-run is fine on 11.0 amd64 and 11.0 i386

Looking at https://pkg-status.freebsd.org I'm not convinced queueing mere 1164 packages is enough to qualify for an exp-run given the unknowns here. Can you do as A/B test on 10.3 without/with the patch, both builds from scratch and against the same revision?
Comment 9 Antoine Brodin freebsd_committer 2016-12-25 09:25:45 UTC
(In reply to Jan Beich (mail not working) from comment #8)
Exp-run results for 11.0 amd64 are available at http://package18.nyi.freebsd.org/jail.html?mastername=110amd64-default-PR215401

Exp-run results for 11.0 i386 are available at http://package18.nyi.freebsd.org/jail.html?mastername=110amd64-default-PR215401
Comment 10 Antoine Brodin freebsd_committer 2016-12-25 09:26:26 UTC
(In reply to Antoine Brodin from comment #9)
Comment 11 Antoine Brodin freebsd_committer 2016-12-30 15:19:56 UTC
Exp-run looks fine,  review still welcome.
Comment 12 Bryan Drewery freebsd_committer 2017-01-03 18:18:18 UTC
This should come out too:

Mk/Scripts/functions.sh:180:              ARCH \
Comment 13 Antoine Brodin freebsd_committer 2017-01-16 19:48:01 UTC
Exp-run has to be done again with the Mk/Scripts/functions.sh change
Comment 16 Eitan Adler freebsd_committer freebsd_triage 2018-05-23 10:27:09 UTC
batch change of PRs untouched in 2018 marked "in progress" back to open.