Page MenuHomeFreeBSD

Prefer ${SRCTOP}/foo over ${.CURDIR}/../../foo and ${SRCTOP}/usr.bin/foo over ${.CURDIR}/../foo for paths in Makefiles.
ClosedPublic

Authored by imp on Mar 9 2017, 6:05 PM.
Tags
None
Referenced Files
F103120849: D9932.id26099.diff
Thu, Nov 21, 7:04 AM
F103120717: D9932.id.diff
Thu, Nov 21, 7:02 AM
F103120716: D9932.id26124.diff
Thu, Nov 21, 7:02 AM
F103120710: D9932.id26193.diff
Thu, Nov 21, 7:02 AM
F103120061: D9932.diff
Thu, Nov 21, 6:51 AM
Unknown Object (File)
Mon, Nov 18, 4:50 PM
Unknown Object (File)
Fri, Nov 15, 4:32 AM
Unknown Object (File)
Fri, Oct 25, 3:47 PM
Subscribers

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

imp edited the test plan for this revision. (Show Details)
imp added a reviewer: bdrewery.
This revision is now accepted and ready to land.Mar 9 2017, 6:16 PM

just my $0.02

usr.bin/bzip2/Makefile
3 ↗(On Diff #26099)

choosing just the first of these to respond to but this applies to ALL the changes here:
In my experience this should be done with a /usr/src/Makefile.paths. or /usr/src/srcpaths.mk
this should be included by some of the .mk files.
All these paths should be in that file in the form of SRCPATH_BZIP2=${SRCTOP}/contrib/bzip2
that way:
1/ when you pull some freebsd code for another project you only have one place to change
2/ you can move things or redefine the way this is done in one place

Also the definition of all these could include ${FOO:tA} (I hope I got that right). we should import gmake's $(realpath..) and $(abspath...) which is much more usable.

usr.bin/bzip2/Makefile
3 ↗(On Diff #26099)

The first is an interesting idea but it's a bit out of scope of this work.
The second, of using :tA everywhere, is understood for why it's wanted but it would slow the build down quite a lot. Our realpath(3) is horribly slow.

usr.bin/bzip2/Makefile
3 ↗(On Diff #26099)

I always hit enter too fast. It was so slow that we made bmake cache the results *per process*. So reusing the same :tA in 1 make execution is fast, but across the entire tree if we applied :tA all over the place it would just smash the namecache and create a ton of extra contention there.

just my $0.02

Nice two cents. Not going to do it since it is beyond the scope of this work, but it is a nice idea down the road. Similar to libnames.mk, it would make things better. But there's a lot of them in the tree, so that's outside the scope.

Torn on the FOO:tA thing since isn't needed, would slow things down and SRCTOP is already an absolute path so it wouldn't change anything. But that's a thought for another time.

In D9932#205427, @imp wrote:

just my $0.02

Nice two cents. Not going to do it since it is beyond the scope of this work, but it is a nice idea down the road. Similar to libnames.mk, it would make things better. But there's a lot of them in the tree, so that's outside the scope.

just an idea for sometime

Torn on the FOO:tA thing since isn't needed, would slow things down and SRCTOP is already an absolute path so it wouldn't change anything. But that's a thought for another time.

I think I overstate it with "everywhere: I think "in selected places" might be a better term. We use the gmake equivalent $(abspath $(somepath)) a lot and it makes things a lot more readable. As for the speed of it, whatever gmake does doesnt seem to take so long as I've never noticed a slowdown.

I think I overstate it with "everywhere: I think "in selected places" might be a better term. We use the gmake equivalent $(abspath $(somepath)) a lot and it makes things a lot more readable. As for the speed of it, whatever gmake does doesnt seem to take so long as I've never noticed a slowdown.

In what way. What things were flakey and how did this fix them?

imp edited edge metadata.
  • Move /etc/ to SRCTOP
  • Convert include over to SRCTOP
  • Fix two CURDIR references in comments that should be SRCTOP
  • Make rescue use SRCTOP
  • Fold with usr.bin
  • Convert gnu/lib to using SRCTOP
  • gnu/usr.bin SRCTOP
This revision now requires review to proceed.Mar 10 2017, 6:25 AM
This revision was automatically updated to reflect the committed changes.