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

Share unix getexepath() definition via src/native #44999

Merged
merged 12 commits into from
Nov 26, 2020
Merged

Share unix getexepath() definition via src/native #44999

merged 12 commits into from
Nov 26, 2020

Conversation

am11
Copy link
Member

@am11 am11 commented Nov 20, 2020

Deduplicate implementations via a new shared location; src/native/common.

cc @janvorli, @jkotas

@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@am11 am11 marked this pull request as ready for review November 20, 2020 20:17
@jkotas jkotas requested a review from janvorli November 22, 2020 02:55
@am11
Copy link
Member Author

am11 commented Nov 23, 2020

Remove unixcoreruncommon static lib dependency 04283a3

@jkotas, now that coreconsole is deleted, the only consumer of unixcoreruncommon static lib is unixcorerun. I was thinking about merging these two projects in a separate PR, but since we need the definition of HAVE_GETAUXVAL (fast path) in unixcorerun.cpp (instead of unixcoreruncommon.cpp) as part of this PR, I have merge it here; one less shared lib / .a (archive) to worry about. :)

Copy link
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

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

LGTM, thank you! I really appreciate all the improvements that you are making in the .NET!

@ghost
Copy link

ghost commented Nov 23, 2020

Hello @janvorli!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost removed the auto-merge label Nov 24, 2020
@janvorli
Copy link
Member

The failures are unrelated, but the libraries tests were timing out, so we don't know their outcome. I think the cause for that timeout was fixed, so let me try to re-run the runtime tests one more time. I'll merge the change if the timeout issue is still there.

@am11
Copy link
Member Author

am11 commented Nov 26, 2020

Thanks. Libraries Test Run release coreclr Linux x64 Debug) failure is #45204.

@janvorli janvorli merged commit 05cd29c into dotnet:master Nov 26, 2020
@am11 am11 deleted the feature/unify-getexepath branch November 26, 2020 12:25
@am11
Copy link
Member Author

am11 commented Nov 26, 2020

Thank you for the reviews, @janvorli. Much appreciate your thoughts with many learning points everytime! 👍

@ghost ghost locked as resolved and limited conversation to collaborators Dec 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants