Add custom tool for packaging as "CLI" package types #174
Conversation
|
Depends on aspnet/BuildTools#101 |
natemcmaster
force-pushed
the
namc/nupkg-type
branch
from
ec49bc1 to
6e86538
Compare
|
cc @bricelam could you review? This is similar to the command packager in EF.Tools |
natemcmaster
force-pushed
the
namc/nupkg-type
branch
from
6e86538 to
36304b2
Compare
| #repack-tools .remove-intermediates target='compile' if='Directory.Exists("src") && !IsLinux' | ||
| @{ | ||
| Dotnet("run -p tools/NuGetPackager -- -c " + E("Configuration") + |
There was a problem hiding this comment.
Why not just use the Sake nuget-pack task?? 
There was a problem hiding this comment.
Trying to avoid putting anything in KoreBuild that will be ripped out when we switch to MSBuild....but actually, maybe I should just override the "pack" target completely now that I've ripped out any "dependency" type packages from this project.
| Dotnet("run -p tools/NuGetPackager -- -c " + E("Configuration") + | ||
| " -o artifacts/build/ "+ | ||
| "-n src/Microsoft.Extensions.SecretManager.Tools/project.nuspec " + |
There was a problem hiding this comment.
project.nuspec makes me smile. (The convention is to name them <packageName>.nuspec)
There was a problem hiding this comment.
.. oh yeah... this started as wishful thinking that CLI itself could do nuspec. Will rename to keep it clearer.
| foreach (var depVersion in GetDependencies(project).OrderBy(p => p.Item1).Select(p => p.Item2)) | ||
| { | ||
| idx++; | ||
| props += ";dep_" + idx + "=" + depVersion; |
There was a problem hiding this comment.
How do these align? By ensure order in project.nuspec matches project.json?
| return nugetPath; | ||
| } | ||
| Console.WriteLine("log : Downloading nuget.exe 3.5.0-rc1".Bold().Black()); | ||
| var response = await new HttpClient().GetAsync("https://dist.nuget.org/win-x86-commandline/v3.5.0-rc1/NuGet.exe"); |
There was a problem hiding this comment.
Can we get this from somewhere that stays up-to-date?
There was a problem hiding this comment.
I suppose we can update to latest once 3.5 ships.
There was a problem hiding this comment.
is possible to make the download url as well as file name configurable?
There was a problem hiding this comment.
YAGNI. We need NuGet ≤ 3.5.0-rc1 to get package type support in nuspec. By the time 3.5.0 rtm or 3.6.0 come out, we'll have removed this anyways. See #179
| ] | ||
| } | ||
| { | ||
| "adx": { // Packages written by the ADX team and that ship on NuGet.org |
There was a problem hiding this comment.
comments in json is not recommended. do we need this?
| } | ||
| } | ||
| }, | ||
| "Default": { // Rules to run for packages not listed in any other set. |
|
|
||
| private IEnumerable<Tuple<string, string>> GetDependencies(ProjectContext context) | ||
| { | ||
| // copied from https://github.com/dotnet/cli/blob/rel/1.0.0/src/dotnet/commands/dotnet-pack/PackageGenerator.cs |
There was a problem hiding this comment.
use a permanent link instead (press 'y' while on the github page)
| var dependencyDescription = context.LibraryManager.GetLibraries().First(l => l.RequestedRanges.Contains(dependency)); | ||
|
|
||
| // REVIEW: Can we get this far with unresolved dependencies | ||
| if (dependencyDescription == null || !dependencyDescription.Resolved) |
There was a problem hiding this comment.
dependencyDescription won't be null. if the first element can't be found, exception will be throw from previous First clause.
| continue; | ||
| } | ||
|
|
||
|
|
| foreach (var depVersion in GetDependencies(project).OrderBy(p => p.Item1).Select(p => p.Item2)) | ||
| { | ||
| idx++; | ||
| props += ";dep_" + idx + "=" + depVersion; |
There was a problem hiding this comment.
these two line can be merged into one line:
props += $";dep_{++idx}={depVersion}";
| props += ";dep_" + idx + "=" + depVersion; | ||
| } | ||
|
|
||
| if (Command.CreateDotNet("build", new[] { project.ProjectFile.ProjectFilePath, "--configuration", config }, configuration: config).Execute().ExitCode != 0) |
There was a problem hiding this comment.
break the line. too long to be reviewed.
| return nugetPath; | ||
| } | ||
| Console.WriteLine("log : Downloading nuget.exe 3.5.0-rc1".Bold().Black()); | ||
| var response = await new HttpClient().GetAsync("https://dist.nuget.org/win-x86-commandline/v3.5.0-rc1/NuGet.exe"); |
There was a problem hiding this comment.
is possible to make the download url as well as file name configurable?
| var pInfo = new ProcessStartInfo | ||
| { | ||
| Arguments = ArgumentEscaper.EscapeAndConcatenateArgArrayForProcessStart(args), | ||
| FileName = await GetNugetExePath() |
There was a problem hiding this comment.
This will only work on Windows, won't it? (cc @pranavkm)
There was a problem hiding this comment.
Yup. Running on xplat would require running it via mono (which we don't require and is known to be problematic).
| } | ||
|
|
||
| #repack-tools .remove-intermediates target='compile' if='Directory.Exists("src") && !IsLinux' |
There was a problem hiding this comment.
@bricelam Yeah, this only runs on Windows
natemcmaster
force-pushed
the
namc/nupkg-type
branch
from
a3f8cf6 to
3b1f650
Compare
Resolves #157. cref https://github.com/aspnet/Coherence-Signed/issues/367
This is throw away work but necessary since CLI won't update to support packing "packageTypes" for project.json, and we're not ready to switch to MSBuild based CLI yet. Will open an issue to track the removal of this tool once that changes.