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

Refactor build targets #13

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

kkm000
Copy link
Contributor

@kkm000 kkm000 commented Dec 9, 2017

Igor, I consolidated the target files for all versions of MSBuild above 4. The way they were used was not consistent anyway, and the changes between different build versions were minimal. In fact, the only change (besides those you made for framework versions above 4.5) was the way the scripts looked for the sn.exe tool and only in case TargetFrameworkSDKToolsDirectory not having been set. I cannot think of a modern (.NET 4) build scenario where it would not.

Also I added a target to add the contract assembly (if it were in fact built, naturally) into the BuiltProjectOutputGroupOutput collection. This way it will be packaged along with the main output assembly. That was the main thing I was going to implement, but I did not want to copy-paste it into 5 versions of the build script, hence the previous change. :)

@kkm000
Copy link
Contributor Author

kkm000 commented Dec 9, 2017

Possible blocker: NuGet/Home#6290. Please hold this change, let's see what the resolution is if any. Either way, the plain old nuget.exe pack x.nuspec seems unavoidable. :(

@kkm000
Copy link
Contributor Author

kkm000 commented Dec 19, 2017

Let me split this change in two. The refactoring part is just a refactoring, and the NuGet part makes little sense before NuGet is fixed, so there is no point to complicate the build (an assembly with contracts cannot be packaged correctly w/o a .nuspec file either way).

MSBuild tools version is generally does not correlate with the Visual
Studio or SDK version, and the changes between different versions of
scripts under Contracts/MsBuild/ were inconsequential anyway. We now
use a common set of build files for all MSBuild versions 4.0 and above.

See dotnet/msbuild#52 (comment)
for the elaboration on the versioning conventions.
@kkm000
Copy link
Contributor Author

kkm000 commented Dec 21, 2017

I removed the new NuGet magic from this change. This is only a refactoring, but a good one, I believe, because the way the contracts are build (with the tools hermetically included in the package) should not even potentially depend on the version of MSBuild the user invokes (which is different for different VS prompts). The only remaining forking is between v3.5 (oh no!) and v4.0 which covers the rest.

You may find it easier to see the changes with ?w=1: https://github.com/Igorbek/CodeContracts.MSBuild/pull/13/files?w=1#diff-cf58b2617b98e43e58ff173634ef603b

I copied the latest files from v15.0 over v4.0, so the set of differences you see is the whole thing. Nothing is really different there, apart from a couple bug fixes in path quoting, change in max tool warninig that happened I think in v12.0, and your own additions on .Net framework version forking. But there was a lot of whitespace added and removed between versions, so it's a mess without the ?w=1 URL trick!

@kkm000 kkm000 changed the title Improve build for new NuGet targets Refactor build targets Dec 21, 2017
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.

1 participant