Simplify the build process & scripts #648

stakx · 2018-07-21T09:01:26Z

No description provided.

kzu · 2018-07-24T03:10:32Z

NuGet.Restore.targets

@@ -1,91 +0,0 @@
-<?xml version="1.0" encoding="utf-8"?>


So glad to see this one go...

kzu · 2018-07-24T03:13:17Z

build.cmd

@@ -1,42 +1 @@
-:: Optional batch file to quickly build with some defaults.


I'm not only in favor of making this minimal, I'm actually conviced it's pointless. I will most likely totally remove build.cmd from the moq/moq repo. I think any .NET developer coming across a root build.proj will know what to do. Moreover, with corebuild.io, you even get dynamic help for its props/targets! See https://github.com/moq/moq/blob/master/build.proj

Tadaa! It's gone completely now. 🍾

It gets even better: It's perfectly possible to get rid of build.proj as well, meaning there will only be a Moq.sln left that MSBuild will picks up automatically (because it is the only project in that directory). So people can build Moq using a simple msbuild, msbuild /t:Configure, msbuild /t:Test, msbuild /t:Pack, whatever. It doesn't get any simpler.

Not a fan of that, because it's not easy (even possible?) to add solution-level targets like Configure or Test. I'd keep build.proj since it's the thing that you can run to get reproducible builds that look and run (almost) exactly as what the CI builds, including all steps it runs, like testing and packaging.

People building from the cmdline aren't the same (I think) than those opening Moq.sln straight-away. So it's fine if they have different "paths". If the sln is inside a src dir, then both can find their sweet spot.

it's not easy (even possible?) to add solution-level targets like Configure or Test

It is. You can define them in Before.Moq.sln.targets. But doing so might not even be needed, MSBuild on a solution appears to be smart enough to just forward any targets to the actual projects.

You can also define dummy targets in Directory.Build.props that each solution project will "inherit" and be able to override as needed.

Agreed however that these approaches are somewhat more limited than what you can do in an actual .proj file (e.g. setting default targets).

I'd keep build.proj since it's the thing that you can run to get reproducible builds that look and run (almost) exactly as what the CI builds

Not sure I understand that argument. The same could be said for building on Moq.sln.

Not a fan of that

I'll leave build.proj for now and see how much more I can simplify things. But if it turns out in the end to just be additional work, or boilerplate, then I'd like to get rid of it.

kzu · 2018-07-24T03:15:46Z

build/Build.proj

@@ -0,0 +1,44 @@
+<Project DefaultTargets="Build;Test;Pack">


I would not move this to a less discoverable location, for the reasons metioned in the previous comment. I would also make this a corebuild.io standard project and get rid of the custom restore.

Right, agreed. If build.cmd indeed is allowed to go away, there should be a build.proj in the root.

kzu · 2018-07-24T03:16:17Z

build/Build.proj

+  </Target>
+
+  <Target Name="Configure">
+    <MSBuild Projects="$(RootDirectory)Moq.sln" Targets="Restore" Properties="Step=1;$(CommonBuildProperties)" />


Not sure what those Step properties passed down are for.

They are a hack to prevent MSBuild from caching information about the invoked projects. The thing is that a Restore will create a .props file under obj/ which includes build files from the NuGet packages (e.g. GitInfo). If you invoke Restore together with another task, MSBuild won't read the auto-generated .props file and won't find the NuGet package-provided targets.

I'd love to get rid of this.

~~Interesting... seems to work now without those! Removed.~~ (Sorry no, this is really needed. Using the CoreBuild SDK doesn't help, either.)

kzu · 2018-07-24T03:17:45Z

build/SignAssembly.props

@@ -0,0 +1,9 @@
+<Project>
+
+  <PropertyGroup>


This may be a bit too much granularity ;)

kzu · 2018-07-24T03:19:13Z

build/xUnit.props

@@ -0,0 +1,31 @@
+<Project TreatAsLocalProperty="TestReportFileName;DotnetErrorCode">


xunit is usually written in all-lowercase ;)

kzu · 2018-07-24T03:20:32Z

build/xUnit.props

+    </Exec>
+    <Warning Condition="'$(DotnetErrorCode)' != '0'" Text="Error in executing dotnet." />
+
+    <Exec Command="dotnet xunit -nobuild -configuration $(Configuration) -noshadow -appveyor -html &quot;$(OutputDirectory)$(TestReportFileName).html&quot; -xml &quot;$(OutputDirectory)$(TestReportFileName).xml&quot;"


Not sure why we need this instead of just using xunit.runners.msbuild and passing in the test assembly...

Because that runner targets only net452. That is, won't be usable on Linux boxes without resorting to Mono. I would love to have a build script that runs on Linux, too.

Btw. I'm looking at replacing dotnet xunit with xunit.runner.console. The former is going away with xUnit 4.

hm. I wonder how many request to run this from Linux, honestly... But fine then!

Well, we're supporting .NET Core so why not do it thoroughly. :)

And it's not just Linux. It's also relevant for those who (accidentally) try to build using dotnet msbuild instead of msbuild. Both should work equally well, IMO.

kzu · 2018-07-24T03:20:49Z

packages.config

@@ -1,8 +0,0 @@
-<?xml version="1.0" encoding="utf-8"?>


kzu · 2018-07-24T03:21:33Z

src/Moq/Moq.csproj

@@ -144,4 +150,11 @@
 		</None>
 	</ItemGroup>

+	<Target Name="SetNuspecProperties" BeforeTargets="GenerateNuspec" DependsOnTargets="GitVersion">


Good upgrade to versioning!

kzu · 2018-07-24T03:22:00Z

src/Moq/Moq.nuspec

@@ -2,7 +2,7 @@
 <package xmlns="http://schemas.microsoft.com/packaging/2011/08/nuspec.xsd">


Next step should be to move to NuGetizer ;)

Will look into it!

Thanks for your detailed review!

kzu · 2018-07-26T23:52:49Z

Looks great @stakx. You are awesome 👍

Apparently, the build script used to compute code coverage, but this has been disabled for a long time and would probably take some work to bring up-to-date again. Let's remove this for now and add it back once we have cleaned up the build process.

SourceLink verification has been disabled because it doesn't work well while one is working locally. Let's remove it and bring it back once we have cleaned up the build process.

The CI server (AppVeyor) is set up to push packages to NuGet, and this is how packages should be published. Most users of this build script wouldn't be able to use `Publish` anyway because they do not know the NuGet API key required for pushing. So, remove this target lest people think that they *should* be able to use it.

The `Build` target does not need to depend on `GitVersion`, since the projects to be built (`Moq.csproj` mainly) already require GitInfo themselves. Additionally, inside `build.proj`, setting `Configure` as an initial project target means that `GitVersion` will already have run before other targets that require it (such as `Package`).

Currently, `build.proj` restores xUnit-related packages which is not actually necessary, since the unit test projects already require them. Moving the responsibility of running tests to the test projects them- selves makes this much clearer and means that the build script won't have to do redundant work.

While we're at it, change the format used for AppVeyor build versions to use something a little more unique. The version scheme used so far frequently caused version collisions (and unnecessary build warnings) e.g. when force-pushing changed commits on a PR branch.

This is the final change that makes an external `nuget` tool obsolete, which lets us remove a ton of build script stuff.

(The reason why we're specifying that SourceLink should not run if building from inside Visual Studio is that SourceLink produces some garbled output in the 'Output' window there that cannot be easily suppressed.)

xUnit 2.4 has abandoned the `dotnet-xunit` CLI tool which we rely on to run our unit tests. We cannot keep using `dotnet-xunit` if we want to be able to upgrade xUnit, so let's get rid of it now. Alternatives considered: * `xunit.runner.msbuild` would seem like the obvious choice, but it is not cross-platform since it only targets .NET 4.5. * `xunit.runner.console` would be the next best thing, since it is as feature-complete as `dotnet-xunit` (e.g. it can generate HTML and XML test run reports and knows about options like `-noshadow`) but it's excruciatingly hard to call, since it comes in various versions and we need to map the current `<TargetFramework>` to an appropriate command line. Unfortunately, MSBuild multi-targeting does not work automatically for `<TargetFrameworks>` and we'd need to perform some MSBuild contorsions / recursion to get there. * `dotnet test` works well with multi-targeting, but doesn't have all options that we've used up until now. Let's go with the last one for simplicity's sake. If we get complaints about missing test run report files, we can go back to the more stren- uous `xunit.runner.console`.

We have got `build.proj` to a point where it does only few things: 1. sets default build targets via `<Project @DefaultTargets>` 2. invokes MSBuild on the solution or projects for each target 3. creates and deletes the `out/` directory 4. specifies the dependencies between the targets All of these except (1) can be done without a dedicated build project, and (1) isn't crucial since we can just set the build targets e.g. in `appveyor.yml`. For command line execution, the MSBuild default target `Build` will often be good enough.

stakx · 2018-07-27T21:27:20Z

phew, I think this is finally done. 😓

stakx force-pushed the build branch 6 times, most recently from 50ab1ef to 8e2c425 Compare July 22, 2018 22:11

kzu reviewed Jul 24, 2018

View reviewed changes

stakx force-pushed the build branch 4 times, most recently from 6816873 to ffa223b Compare July 24, 2018 22:36

stakx added 14 commits July 27, 2018 16:50

Remove code coverage-related debris

b20fb2e

Apparently, the build script used to compute code coverage, but this has been disabled for a long time and would probably take some work to bring up-to-date again. Let's remove this for now and add it back once we have cleaned up the build process.

Remove SourceLink-related debris

cd3b3e8

SourceLink verification has been disabled because it doesn't work well while one is working locally. Let's remove it and bring it back once we have cleaned up the build process.

Remove redundant out/ directory creation

d8eae2c

Remove redundant dotnet runnability check

62cddb4

Delegate Package target to source project

53c3da2

Use Pack target instead of nuget pack

1f64335

Use MSBuild-integrated package restore

58b4c47

This is the final change that makes an external `nuget` tool obsolete, which lets us remove a ton of build script stuff.

Extract GitInfo build parts into GitInfo.props

244dc37

Extract SourceLink build parts into SourceLink.props

59c2c82

(The reason why we're specifying that SourceLink should not run if building from inside Visual Studio is that SourceLink produces some garbled output in the 'Output' window there that cannot be easily suppressed.)

Extract xUnit build parts into xUnit.props

3ae5171

stakx force-pushed the build branch 4 times, most recently from 27dd49c to b7b26db Compare July 27, 2018 19:54

stakx force-pushed the build branch 3 times, most recently from 62f3e38 to f193f62 Compare July 27, 2018 21:06

stakx changed the title ~~WIP: Simplify the build process & scripts~~ Simplify the build process & scripts Jul 27, 2018

stakx added 11 commits July 27, 2018 23:23

Extract strong-naming build parts into SignAssembly.props

22fe1c6

Move Out property to auto-import file

5011e89

Remove build.cmd

c92c81c

Trigger AppVeyor build # update in appveyor.yml

90b946f

Upgrade to xUnit.net 2.4 RTM

d357107

Remove obsolete entries from .gitignore

67c48d5

Normalize whitespace to tabs

900f5a9

Create .props for assembly versioning

745b831

Add CoreBuild Standard badge to README

d09e449

stakx force-pushed the build branch from ad3df21 to d09e449 Compare July 27, 2018 21:24

stakx merged commit dfcbd51 into devlooped:master Jul 27, 2018

stakx deleted the build branch July 27, 2018 21:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplify the build process & scripts #648

Simplify the build process & scripts #648

stakx commented Jul 21, 2018

kzu Jul 24, 2018

kzu Jul 24, 2018

stakx Jul 24, 2018

stakx Jul 24, 2018

kzu Jul 26, 2018

stakx Jul 27, 2018 •

edited

Loading

kzu Jul 24, 2018

stakx Jul 24, 2018

kzu Jul 24, 2018

stakx Jul 24, 2018 •

edited

Loading

kzu Jul 24, 2018

kzu Jul 24, 2018

kzu Jul 24, 2018

stakx Jul 24, 2018

kzu Jul 26, 2018

stakx Jul 27, 2018

kzu Jul 24, 2018

kzu Jul 24, 2018

kzu Jul 24, 2018

stakx Jul 24, 2018 •

edited

Loading

kzu commented Jul 26, 2018

stakx commented Jul 27, 2018

		@@ -1,42 +1 @@
		:: Optional batch file to quickly build with some defaults.

		@@ -0,0 +1,31 @@
		<Project TreatAsLocalProperty="TestReportFileName;DotnetErrorCode">

		@@ -2,7 +2,7 @@
		<package xmlns="http://schemas.microsoft.com/packaging/2011/08/nuspec.xsd">

Simplify the build process & scripts #648

Simplify the build process & scripts #648

Conversation

stakx commented Jul 21, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stakx Jul 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stakx Jul 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stakx Jul 24, 2018 • edited Loading

Choose a reason for hiding this comment

kzu commented Jul 26, 2018

stakx commented Jul 27, 2018

stakx Jul 27, 2018 •

edited

Loading

stakx Jul 24, 2018 •

edited

Loading

stakx Jul 24, 2018 •

edited

Loading