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

Only emit debug info when asked #378

Closed
2 tasks done
antoyo opened this issue Nov 7, 2023 · 11 comments
Closed
2 tasks done

Only emit debug info when asked #378

antoyo opened this issue Nov 7, 2023 · 11 comments
Labels
good first issue Good for newcomers

Comments

@antoyo
Copy link
Contributor

antoyo commented Nov 7, 2023

Currently, debug info is always emmited.

  • context.set_debug_info(true); should not always be called in src/base.rs.
  • -Cdebuginfo=2 should probably not be always added in build_system/src/config.rs.
@antoyo antoyo added the good first issue Good for newcomers label Nov 7, 2023
@mubarak23
Copy link
Contributor

mubarak23 commented Mar 18, 2024

@antoyo please i will love to work on this issue and submit the PR to link as we discuss earlier in the mail

@antoyo
Copy link
Contributor Author

antoyo commented Mar 18, 2024

Good. Feel free to ask any questions if you need help.

@mubarak23
Copy link
Contributor

@antoyo if i understand the config.rs file correctly RUSTFLAGS is what collect all the the environment variables and join them into string, thus removing the "-Cdebuginfo=2" from vector, mean the debug option will not get emitted

@bjorn3
Copy link
Member

bjorn3 commented Mar 19, 2024

The standard library shipped with rustup is built with -Cdebuginfo=1.

@mubarak23
Copy link
Contributor

@bjorn3 i see, does this mean, -Cdebuginfo=1 will not emit debug information, while -Cdebuginfo=2 emit debug information

@bjorn3
Copy link
Member

bjorn3 commented Mar 19, 2024

-Cdebuginfo=1 only emits the line tables part of the debug info (+ some other things), while -Cdebuginfo=2 emits all debug info. Now that I'm taking a closer look, I see that debuginfo is already enabled for the standard library outside of the -Cdebuginfo=2 your PR removes, so I think your PR is fine. (It is enabled for the standard library at

)

@antoyo
Copy link
Contributor Author

antoyo commented Mar 19, 2024

This will emit all debug info, though, so we might as well change this to:

debug = "limited"

@mubarak23: Could you please add this change to your PR?

@mubarak23
Copy link
Contributor

@antoyo i will add it to the PR and push

@mubarak23
Copy link
Contributor

PR updated with changes requested @antoyo

@antoyo
Copy link
Contributor Author

antoyo commented Mar 19, 2024

Thanks a lot! I'll merge it when the CI is done.

@antoyo
Copy link
Contributor Author

antoyo commented Mar 19, 2024

Done in antoyo#1.

@antoyo antoyo closed this as completed Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants