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

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 7 12:31:57 PDT 2020


MaskRay added inline comments.


================
Comment at: lld/test/ELF/reproduce-deplibs.s:13
+# RUN: tar xf repro.tar
+# RUN: diff foo.a repro/%:t.dir/foo.a
+
----------------
andrewng wrote:
> MaskRay wrote:
> > `cmp` may be better for comparison between binary files.
> The other reproduce tests make use of `diff`, so I've just followed their lead. However, `cmp` probably is more appropriate for the binary files. Perhaps we should change them all to use `cmp` instead of `diff` where applicable?
Yeah, I've noticed that. `diff -> cmp` should be fine.

Out of curiosity, how did you notice that reproduce-deplibs.s should be tested as well? 


================
Comment at: lld/test/ELF/reproduce-deplibs.s:1
+# REQUIRES: x86, shell
+
----------------
Can you also delete `shell` and check whether harbomaster (pre-merge testing) complains?


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

https://reviews.llvm.org/D77659





More information about the llvm-commits mailing list