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

Implement native GetModuleIndex #34997

Merged
merged 3 commits into from
Apr 21, 2020

Conversation

janvorli
Copy link
Member

@janvorli janvorli commented Apr 15, 2020

This change replaces managed GetModuleIndex tool by shell scripts using
native tools.

Close #34920

@janvorli janvorli added this to the 5.0 milestone Apr 15, 2020
@janvorli janvorli self-assigned this Apr 15, 2020
@mikem8361
Copy link
Member

Thanks Jan for doing this.

Should the "clr.buildtools" subset be removed now (eng\subsets.props) and the line in eng\pipelines\coreclr\templates\build-job.yml that builds it? And the GetModuleIndex tool source?

I can do that clean if you want.

Copy link
Member

@mikem8361 mikem8361 left a comment

Choose a reason for hiding this comment

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

Look good. Thanks Jan for doing this.

@janvorli
Copy link
Member Author

Should the "clr.buildtools" subset be removed now

Ah, I knew I have forgotten something. Thank you for reminding me that. I'll add the cleanup to this PR.

@janvorli
Copy link
Member Author

@mikem8361 I've removed the clr.buildtools stuff, can you please take a look one more time to make sure I've not forgotten anything?

@mikem8361
Copy link
Member

Some minor things that shouldn't be needed anymore. They are not that important to remove.

eng/Versions.props line 80: remove <MicrosoftFileFormatsVersion>1.0.120601</MicrosoftFileFormatsVersion>

src/coreclr/CMakeLists.txt line 28: remove CLR_DOTNET_COMMAND cmake property

@janvorli
Copy link
Member Author

src/coreclr/CMakeLists.txt line 28: remove CLR_DOTNET_COMMAND cmake property

I've done that already.
I've pushed one more commit to remove the eng/Versions.props line 80.

@mikem8361
Copy link
Member

Thanks Jan. Looks good.

@janvorli janvorli closed this Apr 16, 2020
@janvorli janvorli reopened this Apr 16, 2020
@janvorli janvorli force-pushed the implement-native-getmoduleindex branch from 1e3783c to 0a9e95c Compare April 17, 2020 18:51
This change replaces managed GetModuleIndex tool by shell scripts using
native tools.
@janvorli janvorli closed this Apr 20, 2020
@janvorli janvorli reopened this Apr 20, 2020
@janvorli janvorli force-pushed the implement-native-getmoduleindex branch from 86ad621 to 42ad5e5 Compare April 20, 2020 13:11
@janvorli janvorli merged commit 85aee4d into dotnet:master Apr 21, 2020
@janvorli janvorli deleted the implement-native-getmoduleindex branch April 21, 2020 12:14
@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 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.

CoreCLR should error if clr.buildtools are not built and you try to build a subset that is dependent of that
3 participants