Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make -jN: unpack_snapshot's cleaning can interfere with libuv build products #8865

Closed
pnkfelix opened this issue Aug 29, 2013 · 7 comments
Closed

Comments

@pnkfelix
Copy link
Member

On Mac OS X, when I run time remake --trace -j8 from a clean checkout, three things can occur in parallel:

  1. Running get-snapshot.py, which is largely consumed by downloading the snapshot
  2. Building LLVM
  3. Building libuv and associated products, and copying them into place.

In particular, libmorestack.a is first built as part of libuv and then copied into
x86_64-apple-darwin/stage0/lib/rustc/x86_64-apple-darwin/lib/libmorestack.a

But part of what get-snapshot.py does is to clean out the target area of the unpacking before doing the current unpacking. (I added that code for fixing #3225 .)

And in a sequential build, this cleaning step usually happens before the libuv building steps; but for a parallel build, the steps can be reordered.

The reason the re-ordering matters is: It looks like the net that get-snapshot.py is a bit too broad, and it ends up deleting the libmorestack.a that it had copied over, among other things:

opening snapshot dl/rust-stage0-2013-08-14-e7b5729-macos-x86_64-f44aba76e9d7a9a28b8a6dd78f14576e7c84fbf3.tar.bz2
removing x86_64-apple-darwin/stage0/lib/rustc/x86_64-apple-darwin/lib/libmorestack.a
removing x86_64-apple-darwin/stage0/lib/rustc/x86_64-apple-darwin/lib/librustllvm.dylib
removing x86_64-apple-darwin/stage0/lib/rustc/x86_64-apple-darwin/lib/librustrt.dylib
extracting rust-stage0/bin/rustc
extracting rust-stage0/lib/libstd-6c65cf4b443341b1-0.8-pre.dylib
extracting rust-stage0/lib/libextra-a7c050cfd46b2c9a-0.8-pre.dylib
extracting rust-stage0/lib/librustc-d3cb8c2ccd84a7a7-0.8-pre.dylib
extracting rust-stage0/lib/libsyntax-64629f7f0c6a9bc-0.8-pre.dylib
extracting rust-stage0/lib/librustrt.dylib
extracting rust-stage0/lib/librustllvm.dylib

So, to fix this, either:

  1. get-snapshot.py could be restricted in what paths it deletes; it can't hard-code them, since it needs to match previous snapshot's SHA's, but there is potentially a simpler strategy we could adopt. Or,
  2. The libuv (and perhaps also the llvm) builds could be forced to wait until the snapshot has been unpacked. Or at least the copies of their products into the target area. Or,
  3. We could refactor the cleaning step as a separate action from grabbing the snapshot, and force that to run before all three of the above parallel actions.

Anyway, I've become sort of the local make guru, so I'll see what I can do about this. It probably only affects people who do make -jK for large/interesting K, and the problem often goes away on subsequent runs of make once the prior run's intermediate build products are in place.

@pnkfelix
Copy link
Member Author

part of #8058

@pnkfelix
Copy link
Member Author

Potential fix, following the idea/proposal (2) from the description:

diff --git a/mk/target.mk b/mk/target.mk
index 0d798f4..ac7a1b8 100644
--- a/mk/target.mk
+++ b/mk/target.mk
@@ -32,7 +32,8 @@ define TARGET_STAGE_N

 $$(TLIB$(1)_T_$(2)_H_$(3))/libmorestack.a: \
        rt/$(2)/stage$(1)/arch/$$(HOST_$(2))/libmorestack.a \
-       | $$(TLIB$(1)_T_$(2)_H_$(3))/
+       | $$(TLIB$(1)_T_$(2)_H_$(3))/ \
+         $(HBIN0_H_$(CFG_BUILD_TRIPLE))/rustc$(X_$(CFG_BUILD_TRIPLE))
    @$$(call E, cp: $$@)
    $$(Q)cp $$< $$@

It works by setting up an order-only dependency of the libmorestack.a cp recipe on the same target that drives the snapshot download+clean+unpack process.

Its an ugly hack (which could maybe be made a little bit less ugly by introducing some well-chosen names such as $(SNAPSHOT_UNPACK_PRODUCT) or something), but I am not sure I want to invest the effort to go through the other options I listed.

@pnkfelix
Copy link
Member Author

(the above is on a feasible track, but it is not complete. i might have a PR ready in a while though; want to finish a test of it first this time.)

@pnkfelix
Copy link
Member Author

I might have been wrong to use an order-only dependency here. I'm not sure. I am trying to figure out if the use of an order-only dependency is what injected #9531

(It should be safe to change the dependency into a normal dependency, since the recipe $(SNAPSHOT_RUSTC_POST_CLEANUP): ends with an invocation of touch on the target, so that should stop it from re-running the recipe on future make invocations until the snapshot file is updated.)

@pnkfelix
Copy link
Member Author

(but honestly I'm still not sure why that distinction would cause #9531 ... I'm still inspecting.)

@pnkfelix
Copy link
Member Author

Hmm, the definition of $(SNAPSHOT_RUSTC_POST_CLEANUP) is in stage0.mk, but that file is included after the includes of target.mk and host.mk. I wonder if that's part of the problem.

@pnkfelix
Copy link
Member Author

see PR #9542

flip1995 pushed a commit to flip1995/rust that referenced this issue Jun 4, 2022
Strip `clippy::` prefix from search strings

changelog: none
closes: rust-lang#8865

r? `@xFrednet`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant