msbuild task and cli nugets #1992

majastrz · 2021-03-24T04:21:45Z

Added:

MSBuild task
NuGet packaging for the MSBuild task and CLI tool
E2E tests for NuGet packages.

This fixes #290.

majastrz · 2021-03-24T04:24:42Z

.pipelines/Common.yml

@@ -77,6 +77,109 @@ stages:
        official: ${{ parameters.official }}
        rid: osx-x64

+  - job: packages_windows


packages_windows [](start = 9, length = 16)

Packages have to be signed on a Windows container (limitations of the current infra), but tests have to run on the right OS as separate jobs. This is different than the GH build.

majastrz · 2021-03-24T04:25:17Z

.pipelines/Common.yml

+        rid: win-x64
+        artifactSuffix: windows
+
+  - job: packages_test_linux


packages_test_linux [](start = 9, length = 19)

Sadly no osx image in the internal pipeline, which means no E2E tests :( We do have them on the GH side, though.

majastrz · 2021-03-24T04:25:59Z

scripts/PublishPackages.ps1

@@ -0,0 +1,42 @@
+<#
+Prerequisites:


Prerequisites [](start = 0, length = 13)

Same script as in the types-az repo.

majastrz · 2021-03-24T04:26:24Z

src/Directory.Build.targets

@@ -19,4 +19,30 @@
  <ItemGroup>
    <AdditionalFiles Include="$(MSBuildThisFileDirectory)/BannedSymbols.txt" />
  </ItemGroup>
+
+  <!-- Nuget Publishing related properties that are common to all packages -->


Nuget [](start = 7, length = 5)

Copied from the types-az repo.

majastrz · 2021-03-24T04:27:19Z

src/Bicep.Cli.Nuget/nuget.proj

+  Prerequisites:
+  - Copy bicep.exe and *.pdb from Bicep CLI publish directory to bicep\ directory under this project.
+-->
+<Project Sdk="Microsoft.Build.NoTargets">


NoTargets [](start = 30, length = 9)

This is a completely custom NuGet without a real assembly, so I had to do it myself in a NoTargets SDK project :( Thankfully NuGet.exe is available in a NuGet package and works on all the major OSs.

majastrz · 2021-03-24T04:28:51Z

src/Bicep.MSBuild.E2eTests/examples/Directory.Build.props

+<!--
+  This project prevents the e2e tests from inheriting our repo build settings.
+-->
+<Project>


Project [](start = 1, length = 7)

The e2e tests for the msbuild task involve calling msbuild to build test projects. The files in this directory isolate the test builds from our normal settings.

majastrz · 2021-03-24T04:29:20Z

src/Bicep.MSBuild.E2eTests/examples/fail/fail.proj

+    Pickup latest available packages (including prerelease) from local feed configured in NuGet.config.
+  -->
+  <ItemGroup>
+    <PackageReference Include="Azure.Bicep.CommandLine.$(RuntimeSuffix)" Version="*-*" />


- [](start = 82, length = 3)

This is not ok to use in a normal project, but for test purposes it's a great way of picking up the latest package from the local feed.

majastrz · 2021-03-24T04:30:12Z

src/Bicep.MSBuild.E2eTests/examples/NuGet.config

+  <packageSources>
+    <clear />
+    <add key="nuget.org" value="https://api.nuget.org/v3/index.json" />
+    <add key="Local" value="local-packages" />


local-packages [](start = 28, length = 14)

The GH and ADO build is setup to drop packages here when they are built, so the project will use newest package from the build.

majastrz · 2021-03-24T04:33:33Z

src/Bicep.MSBuild.E2eTests/tsconfig.json

@@ -0,0 +1,15 @@
+{
+    "compilerOptions": {


compilerOptions [](start = 5, length = 15)

I mostly set up the e2e project by copy/pasting existing things. @shenglol and @anthony-c-martin, I haven't done a whole lot of TS, so feel free to point out any weirdness.

anthony-c-martin · 2021-03-24T10:13:56Z

src/Bicep.MSBuild.E2eTests/package.json

+    "@types/jest": "^26.0.21",
+    "cross-spawn": "^7.0.3",
+    "jest": "^26.6.3",
+    "prettier": "^2.2.1",


Are you using prettier in this project?

I am now :)

anthony-c-martin · 2021-03-24T10:25:56Z

src/Bicep.MSBuild.E2eTests/src/example.ts

+    const projectDir = path.normalize(path.join(__dirname, `../examples/${projectName}/`))
+      this.projectDir = projectDir,
+      this.projectFile = path.join(projectDir, `${projectName}.proj`)


[nit] missing semicolons and indenting. Prettier would probably catch this if you feel like enabling it.

Fixed by auto formatting.

anthony-c-martin · 2021-03-24T10:27:30Z

src/Bicep.MSBuild.E2eTests/src/example.ts

+      encoding: "utf-8",
+    });
+
+    if(expectSuccess && result.status != 0) {


Convention is to use === and !== by default unless you've got very good reasons not to

Might be a bit late for this - but is packaging architecture-agnostic managed DLLs an option?

e.g. invoking it via:
dotnet ./src/Bicep.Cli/bin/Debug/net5.0/bicep.dll --help

We'd need a check for dotnet >= 5 somehow (or could perhaps pull it down via nuget); which may or may not be an option for internal MSFT builds.

That was the initial approach I wanted to go with, but users with locked down pipelines do not always control which runtimes are installed on the build agents. I think in the future we can add a platform-agnostic version of the CLI nuget and update the MSBuild nuget to call dotnet on it, but the current CLI nugets will need to remain.

The above reply went to the wrong comment, but fixed the wrong operator also.

anthony-c-martin · 2021-03-24T10:45:27Z

src/Bicep.MSBuild.E2eTests/src/example.ts

+  public clean() {
+    const result = spawn.sync("git", ["clean", "-dfx", "--", this.projectDir], {
+      cwd: this.projectDir,
+      stdio: "pipe",
+      encoding: "utf-8",
+    });
+
+    if(result.status != 0) {
+      this.handleFailure("git", result);
+    }
+  }


We had an issue raised about having an implicit dependency on git in our build process; might be nice to avoid this if possible - for example by staging files to a temp directory.

https://www.npmjs.com/package/nodegit is a potential solution, but I personally hate it - on some platforms it has to compile git from source which takes AGES

Yeah I remember that, but it's rather difficult to get all the dependencies lined up to even run these tests correctly. We can fix once we get feedback.

scripts/UploadSignedReleaseArtifacts.ps1

anthony-c-martin · 2021-03-24T11:09:28Z

src/Bicep.MSBuild.E2eTests/.editorconfig

@@ -0,0 +1,9 @@
+root = true


Worth just adding this to the top-level .editorconfig for [*.{ts,tsx,js}] rather than creating new ones?

Yeah good pt.

anthony-c-martin · 2021-03-24T13:14:05Z

Might be a bit late for this - but is packaging architecture-agnostic managed DLLs an option?

e.g. invoking it via:
dotnet ./src/Bicep.Cli/bin/Debug/net5.0/bicep.dll --help

We'd need a check for dotnet >= 5 somehow (or could perhaps pull it down via nuget); which may or may not be an option for internal MSFT builds.

shenglol · 2021-03-24T18:57:38Z

src/Bicep.MSBuild.E2eTests/src/example.ts

+import * as spawn from 'cross-spawn';
+import * as path from "path";
+import * as fs from "fs";
+import { SpawnSyncReturns } from 'node:child_process';


[nit] Inconsistent quotes. Formatting the document in VSCode will fix these automatically if you have the prettier extension installed.

Fixed by auto formatting.

shenglol · 2021-03-24T18:58:35Z

src/Bicep.MSBuild.E2eTests/src/asserts.ts

@@ -0,0 +1,6 @@
+


Missing license header.

Fixed by auto formatting.

shenglol · 2021-03-24T18:59:13Z

src/Bicep.MSBuild.E2eTests/src/example.ts

@@ -0,0 +1,72 @@
+


Missing license header.

Fixed by auto formatting.

This reverts commit 6452a40.

msbuild task and cli nugets

29a1f54

majastrz commented Mar 24, 2021

View reviewed changes

anthony-c-martin reviewed Mar 24, 2021

View reviewed changes

scripts/UploadSignedReleaseArtifacts.ps1 Show resolved Hide resolved

anthony-c-martin reviewed Mar 24, 2021

View reviewed changes

shenglol reviewed Mar 24, 2021

View reviewed changes

majastrz added 4 commits March 24, 2021 12:27

equals

aa4a299

consolidate editorconfigs

2caecc8

eslint and prettier

ca3d158

add lint to CI

0b07efc

anthony-c-martin approved these changes Mar 25, 2021

View reviewed changes

majastrz merged commit 6452a40 into main Mar 25, 2021

shenglol approved these changes Mar 25, 2021

View reviewed changes

majastrz deleted the majastrz/msbuild branch March 25, 2021 01:10

majastrz added a commit that referenced this pull request Mar 27, 2021

Revert "msbuild task and cli nugets (#1992)"

298572f

This reverts commit 6452a40.

snyk-bot mentioned this pull request Jan 31, 2022

[Snyk] Upgrade winston from 3.3.3 to 3.3.4 CryptoMoneyBotz/bicep#8

Open

snyk-bot mentioned this pull request Jan 31, 2022

[Snyk] Upgrade winston from 3.3.3 to 3.3.4 Manny27nyc/bicep#10

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

msbuild task and cli nugets #1992

msbuild task and cli nugets #1992

majastrz commented Mar 24, 2021 •

edited

Loading

majastrz Mar 24, 2021

majastrz Mar 24, 2021 •

edited

Loading

majastrz Mar 24, 2021

majastrz Mar 24, 2021

majastrz Mar 24, 2021

majastrz Mar 24, 2021

majastrz Mar 24, 2021

majastrz Mar 24, 2021

majastrz Mar 24, 2021

anthony-c-martin Mar 24, 2021

majastrz Mar 24, 2021

anthony-c-martin Mar 24, 2021 •

edited

Loading

majastrz Mar 24, 2021

anthony-c-martin Mar 24, 2021

majastrz Mar 24, 2021

majastrz Mar 24, 2021

anthony-c-martin Mar 24, 2021

majastrz Mar 24, 2021

anthony-c-martin Mar 24, 2021

majastrz Mar 24, 2021

majastrz Mar 24, 2021

anthony-c-martin commented Mar 24, 2021 •

edited

Loading

shenglol Mar 24, 2021

majastrz Mar 24, 2021

shenglol Mar 24, 2021

majastrz Mar 24, 2021

shenglol Mar 24, 2021

majastrz Mar 24, 2021

msbuild task and cli nugets #1992

msbuild task and cli nugets #1992

Conversation

majastrz commented Mar 24, 2021 • edited Loading

Choose a reason for hiding this comment

majastrz Mar 24, 2021 • 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

anthony-c-martin Mar 24, 2021 • 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

anthony-c-martin commented Mar 24, 2021 • 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

majastrz commented Mar 24, 2021 •

edited

Loading

majastrz Mar 24, 2021 •

edited

Loading

anthony-c-martin Mar 24, 2021 •

edited

Loading

anthony-c-martin commented Mar 24, 2021 •

edited

Loading