[PATCH] D54384: [llvm-objcopy] Add --build-id-link-dir flag

[Jake Ehrlich via Phabricator via llvm-commits]

jakehehrlich added inline comments.


================
Comment at: llvm/test/tools/llvm-objcopy/build-id-link-dir.test:3
+# RUN: mkdir -p %t-dir
+# RUN: llvm-objcopy --build-id-link-dir=%t-dir %t %t2
+# RUN: cmp %t-dir/4f/cb712aa6387724a9f465a32cd8c14b %t
----------------
jakehehrlich wrote:
> rupprecht wrote:
> > Does this also work for in-place invocations?
> It works it's just odd because you modify the the thing you sent to the build id directory.
Sorry, this is not right as my tests show. I thought this would be the case but I remembered that FileBuffer atomically renames the file into place which has different semantics. I wrote that comment piror to thinking about that and didn't delete this.


================
Comment at: llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp:144
+  outs() << "out: " << Config.OutputFilename << " Path: " << Path << "\n";
+  auto EC = sys::fs::create_hard_link(Config.InputFilename, Path);
+  if (EC) {
----------------
jakehehrlich wrote:
> rupprecht wrote:
> > Why create a hard link instead of copying the file?
> To save space but I guess copying would make the inplace case more sensible. Generally the build system may still need the unstripped file around. For instance, LLD doesn't support linking against binaries stripped with --strip-sections. Because debug binaries total to a rather large amount, not hard-linking would all at once double the size of the build. Currently in Fuchsia, on a lower layer, our build requires ~30gb of space. Of that 10 gb is for unstripped binaries. If we copied we'd suddenly shoot our build size up to 40gb for the smallest part. This isn't a terrible cost mind you but some people have complained about the space requirements. That seems like a high cost to pay to me.
So also follow up on this comment as well, most of this is still correct but this behavior is actually exactly as sensible as copying and also saves significant space. Also almost all tools use the rename at the end thing needed to make this true to gain atomicity and avoid causing problems on failure. So I think hard linking is just strictly better.


Repository:
  rL LLVM

https://reviews.llvm.org/D54384