Fix copying of validation test files #956
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The files were being copied in such a way that they ended up not only in the framework binaries directory (e.g., bld\bin\Sarif.ValidationTests\AnyCPU_Release\net461, but also in the parent directory (I don't know why). This change not only avoids the extra copy, but is much simpler, and takes advantage of the project system rather than using custom code.
ghost
requested review from
ghost
commented
Jul 23, 2018
| CopyToOutputDirectory="PreserveNewest" /> | ||
| <None Include="$(SolutionDir)\Sarif.FunctionalTests\v1\**\*.sarif" | ||
| CopyToOutputDirectory="PreserveNewest" | ||
| LinkBase="v1" /> |
There was a problem hiding this comment.
LinkBase is a new feature added in .NET SDK 2.0 -- and a good thing, too, because without it, you end up with
OutputPath\
ConverterTestFiles\
...
rather than what you want, which is
OutputPath\
v1\
ConverterTestFiles\
...
See dotnet/msbuild#2949.
ghost
commented
Jul 23, 2018
| @@ -139,6 +139,8 @@ function New-NuGetPackageFromNuspecFile($project, $version, $suffix = "") { | |||
| if ($LASTEXITCODE -ne 0) { | |||
| Exit-WithFailureMessage $ScriptName "$project NuGet package creation failed." | |||
| } | |||
|
|
|||
| Write-Information " Successfully created package '$BinRoot\NuGet\$Configuration\$Project.$version.nupkg'." | |||
There was a problem hiding this comment.
dotnet pack produces this message for NuGet packages built from a project file. But we weren't producing any message for NuGet packages build from a .nuspec file. This message mimics the format (including the indentation) of the message from dotnet pack.
EasyRhinoMSFT
approved these changes
Jul 23, 2018
michaelcfanning
pushed a commit
that referenced
this pull request
Jul 24, 2018
* Issues 176, 149, 158, 187, 172, 175, 178, 189, 191 * Provide an --insert command that accepts a semicolon-delimited list o… (#939) * Provide an --insert command that accepts a semicolon-delimited list of optional data to inject into log files. * Respond to PR feedback * Update comment * Code updates * Correct update script arguments. Put PREfast source code in correct location. (#940) * Merge dammit * Get Multitool Validate verb working. (#931) * Empty solution files. * Successful build of Sarif.dll * Build to common output directory * Sign assemblies. * Set package attributes * Add Sarif to solution files. * Generate version constants and SARIF object model. * Remove SetCurrentVersion.cmd * Bug fix: Don't create existing directory. * Include CommonAssemblyInfo.cs * Clean up Sarif.Converters. * Remove unused usings. * Clean up Sarif.Driver. * Clean up Sarif.TestUtilities * Rearrange lines in Sarif.Driver.csproj * Convert Sarif.UnitTests * GenerateAssemblyInfo=false * Convert Sarif.Convertes.UnitTests. * Convert Sarif.Driver.UnitTests * Convert Sarif.ValidationTests * Get Multitool building with Analyze command * Clean up Sarif.FunctionalTests. * Get SarifValidationSkimmerBase building. * Remove two obsolete rules (only applied to v1). * Convert DoNotUseFriendlyNameAsRuleId (first real rule!) * Fix build break * Delete test assets for obsolete rules. * Add Sarif.Multitool.FunctionalTests * Add /fullclean option to BuildAndTest.cmd * Fix build break by removing analyzers. * Make first two Multitool Analyze functional tests pass. * Functional tests for rule DoNotUseFriendlyNameAsRuleId. * Add /notest option to BuildAndTest.cmd. * Replace Sarif NuGet reference with project reference in Sarif.ValidationTests * Build for multiple platforms; get Multitool.exe building. * Enable Multitool Validate command. * Bug fix: Console warning messages were not * Start validation error codes at 1001. * Add Viewer to solution and get it building * Get appveyor working * FIx up BeforeBuild to get AppVeyor working. * Grr. AppVeyor won't work unless it _invokes_ BeforeBuild. * appVeyor Platform=> Any CPU * AppVeyor: fix typo in RunUnitTests.ps1 * Get unit tests running on AppVeyor * Grr. AppVeyor needs to run the new test script. * RunTests, not RunUnitTests! * RunTests is not a PS script :( * Get tests working in AppVeyor * Get netcoreapp2.0 tests running on AppVeyor * Copy another assembly to test directory * Add <CopyLocalLockFileAssemblies> * Only copy package assemblies for test projects. * Begin refactoring build.props * Move more properties into build.common.props * Move still more properties to build.common.props. * Introduce build.netfx-only.props * Fix analyzer warnings (now treating warnings as errors). * Disable analyzers until I find out how to fix warnings. * Publish Multitool * Remove unneeded build.netfx-only.props. * Build the viewer unit tests. * Comment out viewer tests in a simpler way. * Rename fcib to refs. * Revert "comment out" change: it makes AppVeyor fail. * Get Viewer unit tests working. * Update JSchema: changed rule id prefix. * Get VSIX running by including "suppressed" assemblies. * Delete random generated junk * V2 insert code snippets (#942) * Add file reference class and make preliminary changes * Correct update script arguments. Put PREfast source code in correct location. * Insert single line snippets in rewrite verb * Delete currently unneeded file * Update checked in test samples * Address PR feedback. * Local ToDotNet bits, Revert Uri -> string * Cleanup * File left behind ?? * Remove generated garbage CS files * PR feedback * Float file regions cache notion + basic unit test harness (#945) * Schema + tool update * Finish bringing new build scripts up to parity (#952) This completes the work necessary to bring BuildAndTest.cmd up to parity with its functionality before I started the "clean" conversion to the new project system. Functionally, it adds (or, rather, restores): - NuGet package creation. - The creation of the Signing directory. In addition to the functional changes, I made the following improvements and cleanups: - The scripts are now implemented completely in PowerShell. Please read them as new code. They're really not that long! BuildAndTest.cmd is now just a wrapper around scripts\BuildAndTest.ps1. - I moved the scripts down into a new "scripts" directory, uncluttering the root. - I DRYd out the script code by introducing two PowerShell modules, ScriptUtilities.psm1 and Projects.psm1: - ScriptUtilities.psm1: Implements a couple of generally useful functions. Also defines variables such as `$SourceRoot` and `$BinRoot` that are used throughout the scripts. - Projects.psm1: This module defines two objects, `$Projects` and `$Frameworks`, which enumerate the various kinds of projects we build, and the frameworks for which we build them. Before this, the scripts looped over projects and frameworks "by hand", for example: ``` BuildPackage.cmd Sarif net461 BuildPackage.cmd Sarif.Converters netstandard2.0 ... ``` That meant that if you added a project, you had to modify several places in the scripts. Now you just have to update Projects.psm1. This arrangement has the further advantage of communicating what _kind_ of projects participate in each operation. For example "Create a publish layout directory for every new-style application project". **NOTE**: As I said in email, the package creation is broken, and has been for months. There are .nuspec files in in src\NuGet that are not used, and the packages we are creating don't have the correct dependencies. I filed #951, "NuGet package creation is broken". I also filed #953, "Packaging warnings in BuildAndTest". * Clean up handling of schema file in tests (#954) Sarif.FunctionalTests incorrectly copied the schema to the root of the bld\bin directory rather than to the Sarif.FunctionalTests subdirectory of the bld\bin directory. It turns out that the functional tests don't need the schema at all. I removed the copy task from the project file. Sarif.ValidationTests copied the schema in such a way that not only was there a copy in each framework directory (for example. bld\bin\Sarif.ValidationTests\Any_CPU_Release\net461), there was an extra copy in the "flavor" directory (bld\bin\Sarif.ValidationTests\Any_CPU_Release). I fixed that as well. I looked at this because I wondered why there were copies of the schema directly under bld\bin\AnyCPU_Release. * Restore the ability to create a proper Sarif.Multitool NuGet package. (#955) In my previous change, I removed an unnecessary subdirectory `AnyCPU_Release` from the root of the `bld\bin` directory. Unfortunately, I didn't let `BuildAndTest` run all the way through before I merged, so I didn't notice that this caused the creation of the `Sarif.Multitool` NuGet package to fail. Actually, that package creation was already badly broken, in a couple of ways, and my mistake simply exposed the problem: 1. The `.nuspec` file asked for "all the files under `bld\bin\AnyCPU_Release`". But we were no longer placing _any_ binaries under `bld\bin\AnyCPU_Release`; they were all under individual project directories like `bld\bin\Sarif.Driver\AnyCPU_Release`. The only file directly under `bld\bin\AnyCPU_Release` was the schema file, which didn't belong there at all -- and also, which the `.nuspec` file was explicitly excluding! So when the package creation was "succeeding", it was creating a package with no files in it. 2. The _intent_ of the `.nuspec` file was to gather the entire contents of the `bld\bin` directory, and then _exclude_ the things we didn't want, like test binaries and test content. This was an error-prone process, and what's more, once you'd done it, the `.nuspec` file still had to reach into the `packages` directory to pull out the package binaries that the Multitool needs. But good news! The `dotnet publish` command creates a layout directory that contains _precisely_ all the files that the Multitool needs, and no others. So all the `.nuspec` file has to do is reach into the `Publish` directory. And that's what this change does. * Fix copying of validation test files (#956) The files were being copied in such a way that they ended up not only in the framework binaries directory (e.g., `bld\bin\Sarif.ValidationTests\AnyCPU_Release\net461`, but also in the parent directory (I don't know why). This change not only avoids the extra copy, but is much simpler, and takes advantage of the project system rather than using custom code. Also: * Add a missing build message. * Bring project-file-based Sarif.Sdk packaging to parity with old .nuspec (#958) This change brings the `Sarif.Sdk` NuGet package generated by `dotnet pack` from the `.csproj` file up to parity with the package previously generated by the `.nuspec` file -- which we can now delete. There are two changes: 1. `Sarif.MultiTool` has its own NuGet package, so we don't ship it in the `Sarif.Sdk` package (we used to ship `ConvertToSarif.exe` in the `Sarif.Sdk` package). 2. The `.csproj`-file-based packaging properties don't give us quite the fine-grained control of file placement as the `.nuspec` file did, so the SARIF schema appears in a `Schemata` folder rather than at the root of the package. * Populate all region properties (#957) * Provide an --insert command that accepts a semicolon-delimited list of optional data to inject into log files. * Float file regions cache notion + basic unit test harness * Respond to PR feedback * Fully populate region objects, including snippets * Add 'Regions' to optionally emitted data enum * PR feedback * Fix tests * Update binskim test file (#962) * Provide an --insert command that accepts a semicolon-delimited list of optional data to inject into log files. * Float file regions cache notion + basic unit test harness * Respond to PR feedback * Fully populate region objects, including snippets * Add 'Regions' to optionally emitted data enum * PR feedback * Fix tests * Update BinSkim test file to current schema to assist in schema testing on schemastore.org
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The files were being copied in such a way that they ended up not only in the framework binaries directory (e.g.,
bld\bin\Sarif.ValidationTests\AnyCPU_Release\net461, but also in the parent directory (e.g.,bld\bin\Sarif.ValidationTests\AnyCPU_Release). I don't know why.This change not only avoids the extra copy, but is much simpler, and takes advantage of the project system rather than using custom code.