[PATCH] D77659: [ELF][test] Add reproduce test for dependent libraries

Wed Apr 8 03:44:21 PDT 2020

grimar added inline comments.

================
Comment at: lld/test/ELF/reproduce-deplibs.s:12
+# RUN: ld.lld bar.o -o /dev/null --reproduce repro.tar
+# RUN: tar xf repro.tar
+# RUN: cmp foo.a repro/%:t.dir/foo.a
----------------
jhenderson wrote:
> andrewng wrote:
> > grimar wrote:
> > > jhenderson wrote:
> > > > Idle musing, feel free to ignore: I wonder whether we really need to check that the archive in the tar file is binary identical to the input one? Would just checking that the file is in the package be sufficient?
> > > I also wonder about it.
> > > 
> > > Our `reproduce-windows.s` has:
> > > ```
> > > # RUN: tar tf repro.tar | FileCheck %s
> > > 
> > > # CHECK: repro/response.txt
> > > # CHECK: repro/{{.*}}/build/foo.o
> > > ```
> > I've basically followed the convention of the other reproduce tests (except the Windows specific ones) that all check the contents too. I'm guessing this is just to be sure that it's also the correct file. I'm happy to change it to only test for its presence in the TAR file if that's the consensus.
> I don't really have an opinion either way. Happy to defer to others on this one.
I'd slightly prefer to avoid checking the content because LLD tests are already take some time to pass and would be good to reduce this time. I do not mind to leave it for a follow-up, but see my question above.

================
Comment at: lld/test/ELF/reproduce-deplibs.s:5
+## Windows as the extraction of the tar archive can cause problems related to
+## path length limits.
+
----------------
Does it mean that using `tar tf` like `reproduce-windows.s` do would allow this to be enabled on windows?

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77659/new/

https://reviews.llvm.org/D77659