Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve emar wrapper script #8101

Merged
merged 2 commits into from Feb 15, 2019
Merged

Improve emar wrapper script #8101

merged 2 commits into from Feb 15, 2019

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Feb 15, 2019

  • Make it flake8 clean
  • Re-use DEBUG variable from shared.py
  • Log full command lines in DEBUG mode.
  • Don't assume that the archive file name ends with .a
  • Don't print backtrace when underlying llvm-ar returns non-zero

Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm (although I don't think I've ever seen 'a' or 'b' used, and reading the docs now, still not sure what they do...)

@sbc100
Copy link
Collaborator Author

sbc100 commented Feb 15, 2019

The 'a' and 'b' modifiers means that there is an extra argument before the name of the archive. Previously we were looking at the filename to determine where the output archive name appears on the command line. Now we basically parse the initial args so we know for sure, regardless of the name.

- Make it flake8 clean
- Re-use DEBUG variable from shared.py
- Log full command lines in DEBUG mode.
- Don't assume that the archive file name ends with `.a`
- Don't print backtrace when underlying llvm-ar returns non-zero
@kripken
Copy link
Member

kripken commented Feb 15, 2019

Thanks, sounds good.

@sbc100 sbc100 merged commit 965bdd9 into incoming Feb 15, 2019
@sbc100 sbc100 deleted the cleanup_emar branch February 15, 2019 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants