[PATCH] D88877: [ELF] Drop --whole-archive before processing dependent libraries.

[ben via Phabricator via llvm-commits]

bd1976llvm added inline comments.


================
Comment at: lld/test/ELF/deplibs-ignore-whole-archive.s:7
+## with the "duplicate symbol" error even if a dependent library is referenced
+## more than once, which is very common with them.
+
----------------
bd1976llvm wrote:
> ikudrin wrote:
> > bd1976llvm wrote:
> > > If we implement deduplication for dependent libraries then this test may spuriously pass. I would rewrite the test so that it uses --trace to check that unreferenced archive members are not added to the link. I would also add a comment to say that --whole-archive applying to dependent libraries is confusing and build-systems are not always able to add an option (--no-whole-archive) to the very end of the command line.
> > I have extended the comment. As for a different implementation which might also make the test pass, I do not think the way the issue is fixed really matters. Note also that the test already checks that `bar`, the symbol which is not referenced, is not included in the output. What advantages does checking for absence with `--trace` have?
> Sorry, I misread the test. You can ignore my --trace comment. Checking for the presence of symbol "bar" is sufficient. However, I would remove the parts of the test that check the behaviour for duplicate libraries on the command line as checking for "bar" is sufficient to show that --whole-archive is not active. Also, there is no need to describe the effect of --whole-archive.
> 
> You can rewrite the comment like this...
> 
> ## Check that the dependent libraries are processed in --no-whole-archive mode
> ## even if there is an unterminated --whole-archive on the command line. This 
> ## behaviour is desirable because, applying --whole-archive to the "invisible" dependent
> ## libraries is confusing, and not all build-systems support allowing developers to add
> ## --no-whole-archive to the very end of the command line .
> 
> ...and remove one of the  ".asciz  "lib.a" lines."
My "##"s became 1., 2. 3. :(


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

https://reviews.llvm.org/D88877