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

Fix imported location target property when target contains dashes and CORROSION_RESPECT_OUTPUT_DIRECTORY is true #322

Merged
merged 1 commit into from
Feb 17, 2023
Merged

Fix imported location target property when target contains dashes and CORROSION_RESPECT_OUTPUT_DIRECTORY is true #322

merged 1 commit into from
Feb 17, 2023

Conversation

tronical
Copy link
Contributor

The regex to determine the name of the actual library target tries to strip the -static or -shared suffix did not allow for dashes in the name. Since the regexp closes with a $, the suffix should only match at the very end and we can allow any character before the last dash.

Fixes #321

… CORROSION_RESPECT_OUTPUT_DIRECTORY is true

The regex to determine the name of the actual library target tries to
strip the -static or -shared suffix did not allow for dashes in the
name. Since the regexp closes with a $, the suffix should only match at
the very end and we can allow any character before the last dash.

Fixes #321
@tronical
Copy link
Contributor Author

The test passes for me locally on macOS. Is this known to be a flakey test somehow?

Copy link
Collaborator

@jschwe jschwe left a comment

Choose a reason for hiding this comment

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

Looks good - Not sure what went wrong with the failed test, but it looks like just a flaky failure that is probably not related to corrosion.

@jschwe jschwe merged commit 67232bc into corrosion-rs:master Feb 17, 2023
@tronical tronical deleted the simon/fix-import-location-on-target-with-dash branch February 17, 2023 12:32
@tronical
Copy link
Contributor Author

Thanks for the review & merge. Would it be possible to tag a new release of Corrosion that includes this? We'd love to use a tag instead of a sha1 :)

@jschwe
Copy link
Collaborator

jschwe commented Feb 17, 2023

Would it be possible to tag a new release of Corrosion that includes this?

Sure, I released v0.3.3 and updated v0.3 to point to v0.3.3.

@tronical
Copy link
Contributor Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants