Filter libtool warning about table of contents#15325
Filter libtool warning about table of contents#15325cpsauer wants to merge 2 commits intobazelbuild:masterfrom
Conversation
The warning is unlikely to indicate a real issue, and Bazel already silences parallel warnings with `-no_warning_for_no_symbols`. For more context, see bazelbuild#4057
|
(Tagging @keith, since this came out of our discussion.) |
| 2> >(grep -v "the table of contents is empty (no object file members in the"` | ||
| `" library define global symbols)$" >&2) |
There was a problem hiding this comment.
does this grep not matching cause an unexpected failure?
There was a problem hiding this comment.
I think you could also be less strict here if you wanted
There was a problem hiding this comment.
When running, I'm seeing things succeed fine when the grep doesn't match. Does that resolve the concern?
(I think your concern is a non-zero exit code from grep leading to a non-zero exit code of the script.)
There was a problem hiding this comment.
(I kinda like the full thing to self-document the warning, but can remove if you want.)
There was a problem hiding this comment.
Yes that resolves the concern, I was worried grep would still error. it looks like it's because of the -v that it doesn't?
There was a problem hiding this comment.
Yep, I think so. I was concerned about that, too, when writing
| fi | ||
|
|
||
| WRAPPER="${MY_LOCATION}/xcrunwrapper.sh" | ||
| function libtool() { |
There was a problem hiding this comment.
probably worth naming this something slightly different to avoid confusion that we're calling libtool directly, maybe libtool_func (or ideally something better)
There was a problem hiding this comment.
Would invoke_libtool be okay?
|
All checks are green and pass on the first round. I'll make that change Are we all good to go, then, @keith? Thanks for being so lightning fast and always a delight to work with. |
In response to @keith's good feedback.
keith
left a comment
keith
left a comment
There was a problem hiding this comment.
Yep I think this is safe to filter out! Thanks for the positivity! Someone from google will still have to review and land
sgowroji
added
team-Rules-CPP
|
Wahoo! Thanks @oquenchil x2 |
|
@bazel-io flag |
bazel-io
added
the
potential release blocker
|
@brentleyjones, could I ask what the tags, like yours above, mean? I keep seeing them and am curious :) |
|
@cpsauer I'm flagging them as desired to get into 5.2. If the release manager agrees they do the fork command to create an issue to track the inclusion. After that issue is made someone (preferably the author) can make a cherry-pick PR. I think now that the branch exists the author can just make the PR though. Since I might not be able to make the PR immediately, I like to flag ones I think would be great candidates for the next release. |
|
Got it! Thanks for a great explanation, @brentleyjones. Honored to have made your cut :) |
|
@bazel-io fork 5.2.0 |
bazel-io
removed
the
potential release blocker
The warning is unlikely to indicate a real issue, and Bazel already silences parallel warnings with `-no_warning_for_no_symbols`. For more context, see #4057 Fixes #4057 Closes #15325. PiperOrigin-RevId: 446659148 Co-authored-by: Christopher Sauer <[email protected]>
The warning is unlikely to indicate a real issue, and Bazel already silences parallel warnings with `-no_warning_for_no_symbols`. For more context, see #4057 Fixes #4057 Closes #15325. PiperOrigin-RevId: 446659148 Co-authored-by: Christopher Sauer <[email protected]>
ShreeM01
removed
the
awaiting-review
8 participants
The warning is unlikely to indicate a real issue, and Bazel already silences parallel warnings with
-no_warning_for_no_symbols.For more context, see #4057
Fixes #4057