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

Use SDK-style projects #471

Merged
merged 11 commits into from
May 3, 2019
Merged

Use SDK-style projects #471

merged 11 commits into from
May 3, 2019

Conversation

sbomer
Copy link
Member

@sbomer sbomer commented Feb 20, 2019

This updates projects to use the new simplified format. It depends on @jbevain's branch that I believe is a WIP: https://github.com/jbevain/cecil/tree/new-sdk.

This change will not be ready to merge until the cecil change is ready, but I wanted to share what it's going to look like and get early feedback.

Part of #452.

Edit: now that cecil has been updated to use SDK-style projects, this can move forward. Waiting on #489 to avoid merge conflicts in that PR.

@sbomer sbomer requested a review from marek-safar as a code owner February 20, 2019 22:19
marek-safar pushed a commit that referenced this pull request Feb 26, 2019
Until I can make more progress on SDK-style projects (see #471), I'll begin trying to set up a build of illink using arcade and the old-style projects. In this change I'm getting rid of the scattered linker.sln files in favor of monolinker.sln for monolinker, and illink.sln for the illink/ILLink.Tasks build.
@sbomer sbomer force-pushed the sdkProjects branch 4 times, most recently from 17202d8 to 7f8ea93 Compare March 27, 2019 01:42
@sbomer sbomer changed the title [nomerge] Use SDK-style projects Use SDK-style projects Mar 27, 2019
@sbomer
Copy link
Member Author

sbomer commented Mar 27, 2019

@marek-safar PTAL

@@ -1,13 +1,27 @@
<Project>

<!-- Cecil's Directory.Build.Props imports this file if it
Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't modify the file here. It's submodule copy not actual location to make the changes

Copy link
Member Author

Choose a reason for hiding this comment

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

The contents of this file are purely to adapt the cecil project to our build (our directory layout, our use of arcade), which is the intended purpose of this import in cecil: https://github.com/jbevain/cecil/blob/master/Directory.Build.props#L24. I don't think any of these modifications belong in the cecil project.

<AssemblyName Condition=" ! $(ILLinkBuild) ">monolinker</AssemblyName>
<AssemblyName Condition=" $(ILLinkBuild) ">illink</AssemblyName>
<TargetFrameworkVersion>v4.6.2</TargetFrameworkVersion>
<Copyright>(C) 2006, Jb Evain</Copyright>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use .NET Foundation for the copyright

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm happy to change the copyright info, but I'd rather do it in a separate PR. Here my goal is just to use the new SDK while reproducing the original build as closely as possible.

@mrvoorhe
Copy link
Contributor

@marek-safar @sbomer Would it be OK if we hold off merging this PR until after the PR's that I have open are merged? I suspect this change is going to cause me some issues I will have to work through. Getting the improvement PR's landed and pulled into our UnityLinker is a higher priority for me than patching our projects back together. If this PR lands first I'll have to start cherry picking around this change. Not the end of the world, but if there's no rush it will make my life easier.

@sbomer
Copy link
Member Author

sbomer commented Mar 27, 2019

@mrvoorhe that's fine by me. Please let me know when would be a good time.

@sbomer sbomer changed the title Use SDK-style projects [nomerge] Use SDK-style projects Mar 27, 2019
sbomer added 11 commits May 2, 2019 09:52
Separate out the illink build properties from the monolinker
properties. Also disable building illink for net46 on unix/core
msbuild, which doesn't work due to missing reference assemblies. This
check is more specific than what we used to have, so that illink can
potentially be built with the mono runtime.

Building this project with "nuget restore" and "msbuild" will only
work with the default configuration because nuget restore does not set
the configuration and would see a different target framework from msbuild.
Also add a missing release config for cecil
@sbomer
Copy link
Member Author

sbomer commented May 2, 2019

@mrvoorhe is now a better time to make this change? This will unblock updates to cecil like #540.

@sbomer sbomer mentioned this pull request May 2, 2019
@sbomer sbomer changed the title [nomerge] Use SDK-style projects Use SDK-style projects May 2, 2019
@sbomer
Copy link
Member Author

sbomer commented May 2, 2019

@marek-safar PTAL

@mrvoorhe
Copy link
Contributor

mrvoorhe commented May 3, 2019

Go for it. Thanks for checking.

@marek-safar marek-safar merged commit 6280036 into dotnet:master May 3, 2019
sbomer added a commit to sbomer/linker that referenced this pull request May 14, 2019
With the latest cecil update in
dotnet#471, we are using SDK-style
projects and building cecil for netstandard2.0, so the workaround for
dotnet/sdk#3194 is no longer necessary.
sbomer added a commit to sbomer/linker that referenced this pull request May 14, 2019
With the latest cecil update in
dotnet#471, we are using SDK-style
projects and building cecil for netstandard2.0, so the workaround for
dotnet/sdk#3194 is no longer necessary.
marek-safar pushed a commit that referenced this pull request May 15, 2019
With the latest cecil update in
#471, we are using SDK-style
projects and building cecil for netstandard2.0, so the workaround for
dotnet/sdk#3194 is no longer necessary.
@sbomer sbomer mentioned this pull request May 22, 2019
tkapin pushed a commit to tkapin/runtime that referenced this pull request Jan 31, 2023
Until I can make more progress on SDK-style projects (see dotnet/linker#471), I'll begin trying to set up a build of illink using arcade and the old-style projects. In this change I'm getting rid of the scattered linker.sln files in favor of monolinker.sln for monolinker, and illink.sln for the illink/ILLink.Tasks build.

Commit migrated from dotnet/linker@1331119
tkapin pushed a commit to tkapin/runtime that referenced this pull request Jan 31, 2023
* Use SDK-style projects for linker and tests

* Clean up linker project file

Separate out the illink build properties from the monolinker
properties. Also disable building illink for net46 on unix/core
msbuild, which doesn't work due to missing reference assemblies. This
check is more specific than what we used to have, so that illink can
potentially be built with the mono runtime.

Building this project with "nuget restore" and "msbuild" will only
work with the default configuration because nuget restore does not set
the configuration and would see a different target framework from msbuild.

* Fix assembly title and description for illink

* Fix debug and optimization info for illink configurations

* Remove references to old illink solutions

* Update cecil submodule to latest mono/cecil

* Fix typo

* Update cecil configurations in illink.sln

* Fix cecil strongname build failure with arcade

Work around dotnet/arcade#2321

* Remove unused configs from monolinker.sln

Also add a missing release config for cecil

* Set PublicKey and PublicKeyToken in cecil overrides


Commit migrated from dotnet/linker@6280036
tkapin pushed a commit to tkapin/runtime that referenced this pull request Jan 31, 2023
With the latest cecil update in
dotnet/linker#471, we are using SDK-style
projects and building cecil for netstandard2.0, so the workaround for
dotnet/sdk#3194 is no longer necessary.


Commit migrated from dotnet/linker@56ec756
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants