Page MenuHomeFreeBSD
Differential D2144

Move dtrace tools to cross-tools
ClosedPublic
Actions

Authored by emaste on Mar 25 2015, 8:22 PM.
Tags
None
Referenced Files
F105944104: D2144.id8767.diff
Sun, Dec 22, 10:20 PM
Unknown Object (File)
Mon, Nov 25, 8:24 PM
Unknown Object (File)
Sat, Nov 23, 1:02 PM
Unknown Object (File)
Nov 20 2024, 7:27 AM
Unknown Object (File)
Nov 14 2024, 4:34 AM
Unknown Object (File)
Nov 4 2024, 12:44 PM
Unknown Object (File)
Nov 4 2024, 12:44 PM
Unknown Object (File)
Nov 4 2024, 12:44 PM
View All Files
Subscribers
gonzo
ngie

Details

Reviewers
markj
imp
Commits
rS288069: Move DTrace tools to cross-tools stage
Summary

libelf and libdwarf need their headers installed, and that implies the legacy stage at the moment.

The dtrace tools are needed for the build only, so they can move to the cross tools stage.

Posting for review of the approach for now; some tweaking may be necessary.

Diff Detail

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

Event Timeline

emaste updated this revision to Diff 4421.Mar 25 2015, 8:22 PM
emaste retitled this revision from to Move libelf and libdwarf to legacy, and dtrace tools to cross-tools.
emaste updated this object.
emaste edited the test plan for this revision. (Show Details)
emaste added a reviewer: imp.
emaste added subscribers: ngie, markj.
imp accepted this revision.Mar 25 2015, 8:41 PM
imp edited edge metadata.

Modulo a couple of comments, and maybe an if simplification, this looks good to me.

Makefile.inc1
1238 ↗(On Diff #4421)

How we've handled this in the past is to have 'reach over' makefiles in the elf toolchain libs. However, I'm not seeing an easy way to do that which doesn't replicate a bunch of directories / makefiles, so this should be good.

1455 ↗(On Diff #4421)

So there's two issues here. One is that we can't use the host's ctfmerge tools if they are too old. the other issue is that we can't use it at all if the target arch is different.

I don't think the MACHINE != TARGET test is needed at all. We can successfully build pc98 kernels with i386 ctfmerge tools. That's the only case where MACHINE_ARCH == TARGET_ARCH where MACHINE != TARGET.

This would save a wrapped if which is kinda ugly the way this is wrapped too :)

Might want a quick comment to this effect too:

  1. Dtrace tools are required when the host's tools are too old, or when
  2. we're on a different architecture from the target.
This revision is now accepted and ready to land.Mar 25 2015, 8:41 PM
emaste updated this revision to Diff 4423.Mar 25 2015, 9:14 PM
emaste edited edge metadata.

Update comment and remove unnecessary .if, per @imp

This revision now requires review to proceed.Mar 25 2015, 9:14 PM
ngie added inline comments.Mar 25 2015, 11:45 PM
Makefile.inc1
1238 ↗(On Diff #4423)
  • Can these be built in parallel?
  • Can this list be parameterized out, like I did with buildtools?
1462–1463 ↗(On Diff #4423)

Where are lib/libdwarf and lib/libelf built via build-tools (or is it cross-tools, I'm so confused...)?

emaste added inline comments.Mar 26 2015, 1:50 PM
Makefile.inc1
1238 ↗(On Diff #4423)

Propose a follow-on patch perhaps?

1462–1463 ↗(On Diff #4423)

With this change they are built during legacy, so these dependencies are incorrect. I think there's no need for an explicit dependency any longer.

imp added inline comments.Mar 26 2015, 3:54 PM
Makefile.inc1
1238 ↗(On Diff #4423)

don't worry about building them in parallel. Please. Don't.
There's no need, and this list should be tiny. If it needs to be parallelized, we're doing something horribly wrong.
the legacy stage needs to be absolutely bulletproof. Don't do anything to make it less so.

ngie mentioned this in D2109: Always build the elf toolchain, ctfconvert, et al as part of buildtools.May 23 2015, 4:45 AM
emaste updated this revision to Diff 7882.Aug 11 2015, 5:40 PM
emaste edited edge metadata.

Rebase after committing the libelf move

imp accepted this revision.Aug 11 2015, 5:51 PM
imp edited edge metadata.

This looks good to go.
Still don't think the TARGET != MACHINE test is needed, but I couldn't convince myself that there were no cases where it was needed.
Maybe it still is for the case where you have an up-to-date build box, but you have old sources / binaries that you are upgrading so the
ctf tools in that scenario aren't up to date, and this test forces it always. But I just wonder how it is different than other things that are
unconditionally built in the cross tools section... It is most like the btxld, which has similar tests.

I guess this is a long way of saying that this will work, but there might be a simpler way to accomplish the same thing, but there might
not so go ahead :)

This revision is now accepted and ready to land.Aug 11 2015, 5:51 PM
emaste added a subscriber: gonzo.Aug 11 2015, 6:30 PM
In D2144#68384, @imp wrote:

This looks good to go.
Still don't think the TARGET != MACHINE test is needed, but I couldn't convince myself that there were no cases where it was needed.

Yeah, I'm not certain either. It was added in rS233416 by @gonzo.

emaste updated this revision to Diff 8767.Sep 15 2015, 3:38 PM
emaste retitled this revision from Move libelf and libdwarf to legacy, and dtrace tools to cross-tools to Move dtrace tools to cross-tools.
emaste added a reviewer: markj.
emaste removed a subscriber: markj.

Rebase after markj@ change
libelf and libdwarf change already committed

This revision now requires review to proceed.Sep 15 2015, 3:38 PM
emaste added inline comments.Sep 15 2015, 3:39 PM
Makefile.inc1
1397 ↗(On Diff #8767)

This line will also be deleted.

markj accepted this revision.Sep 15 2015, 5:53 PM
markj edited edge metadata.

Looks reasonable to me.

This revision is now accepted and ready to land.Sep 15 2015, 5:53 PM
Closed by commit rS288069: Move DTrace tools to cross-tools stage (authored by emaste). · Explain WhySep 21 2015, 4:07 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
head/
19 lines

Diff 8864

View Options

head/Makefile.inc1

Loading...