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.

Sign up for GitHub

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

Jump to bottom

Use cargo --unit-graph to figure out whether to link debuginfo #47

Merged
merged 5 commits into from
Nov 27, 2023
Merged

Use cargo --unit-graph to figure out whether to link debuginfo #47

merged 5 commits into from
Nov 27, 2023

Conversation

ian-h-chamberlain
Copy link
Member

Maybe closes #14 ? I'm not positive...

With --unit-graph we can figure out whether ctru-sys will be built with debug symbols or not, and pass the appropriate -l flags in cargo-3ds.

Hopefully, this will always link the proper version of the library and we won't run into linker issues like these again, but it might need a bit of usage to work out the kinks.

@ian-h-chamberlain
Use cargo --unit-graph to figure out debuginfo
545d853 
We can check if `ctru-sys` is being built with debuginfo this way, and
use that to build the stdlib with the same linker flags as ctru-sys is
expected to use.
@ian-h-chamberlain
Handle --unit-graph errors better
e1cf233
@ian-h-chamberlain ian-h-chamberlain requested a review from a team as a code owner November 19, 2023 19:42
@ian-h-chamberlain ian-h-chamberlain mentioned this pull request Nov 19, 2023
Merged
2 tasks
Meziu
Meziu approved these changes Nov 20, 2023
View reviewed changes
Copy link
Member

@Meziu Meziu left a comment

Choose a reason for hiding this comment

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

LGTM, we had discussed about this possibility already, and it seems like a suitable solution.

src/graph.rs Show resolved Hide resolved
@Meziu
Copy link
Member

Meziu commented Nov 23, 2023

Btw, this might warrant a new release on crates.io, since it has changes regarding the whole toolchain.

@ian-h-chamberlain
Copy link
Member Author

Btw, this might warrant a new release on crates.io, since it has changes regarding the whole toolchain.

Good point, do you think we should bump the version to 0.2.0 for this?
For publishing, have you been doing it manually? I think I might be able to do it since I'm in the rust3ds/active group but I haven't tried it yet. I can bump the version in this PR and try to make a release once it's merged, I guess?

@Meziu
Copy link
Member

Meziu commented Nov 25, 2023

Good point, do you think we should bump the version to 0.2.0 for this?

Nah, I don’t think it’s necessary. Just bump the patch number.

For publishing, have you been doing it manually? I think I might be able to do it since I'm in the rust3ds/active group but I haven't tried it yet. I can bump the version in this PR and try to make a release once it's merged, I guess?

Yeah, I’ve been doing it manually. Everybody in the team group should be able to publish.

@ian-h-chamberlain ian-h-chamberlain merged commit f6b9c6d into master Nov 27, 2023
3 checks passed
@ian-h-chamberlain ian-h-chamberlain deleted the feature/link-debuginfo-profile branch November 27, 2023 01:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Meziu Meziu Meziu approved these changes

@AzureMarker AzureMarker Awaiting requested review from AzureMarker AzureMarker is a code owner automatically assigned from rust3ds/active

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Is it necessary to modify RUSTFLAGS in cargo-3ds?
2 participants
@ian-h-chamberlain @Meziu