clairernovotny / ReferenceGenerator

MIT License
75 stars 13 forks source link

Netstandard support #21

Closed scottdorman closed 8 years ago

scottdorman commented 8 years ago

The logic and complexity around figuring out the project information has been moved in to a new Project class. This keeps it all self-contained and lets the engine work with csproj/vbproj/xproj files and lets it use the information in the project files to figure out the path to the binary. I also added a new method to ProjectEngine called GetProjectPackages which takes a Project instance instead of a path to the project.lock/packages.config file so the engine has the responsibility of figuring out which method to call (GetProjectJsonPackages or GetPackagesConfigPackages).

All of this also let me simplify the command line args by dropping the fourth argument. Arg 3 is still a semi-colon delimited list, but it can be a list of just project files "tests\SampleProjJsonPcl\SampleProjJsonPcl.csproj;" or project file and configuration '"tests\SampleProjJsonPcl\SampleProjJsonPcl.csproj=Debug;"

clairernovotny commented 8 years ago

Cool, thx, will take a look in a little bit!

clairernovotny commented 8 years ago

One question on this so far -- because it tries to calculate the binary path from the project file, how would that work if the OutputPath was changed externally - like calling msbuild /p:OutputPath="c:\foo"?

Right now, this is handled by the delayed expansion of $(TargetPath) which is passed in as a parameter to the tool:

https://github.com/onovotny/ReferenceGenerator/blob/netstandard-support/src/ReferenceGenerator/NuSpec.ReferenceGenerator.targets#L6-L11

scottdorman commented 8 years ago

Hmmm...good point. I don't think my change would handle that. Maybe I misunderstood what those two command line args were for then. I thought they worked like this:

args[3] was the path the csproj file and args[4] was the path to the compiled binary that was being included.

Given that understanding, it made sense to me to pull that path from the project file (not taking in to account that it could be changed via msbuild command line). I should be able to change this so it takes the binary file path as the second part of the pair (instead of pulling it from the project file) instead without much effort and I may even be able to make it "smart" enough to handle making the binary path optional (in which case it would pull from the project file.)

clairernovotny commented 8 years ago

Yep, your understanding is correct, it is to get the compiled binary(s) being included. I just couldn't come up with a reliable way of getting them statically from the project files. That's why I had them as a parameter (that was for the most part automatic in the targets.)

If you could please update the PR to pull in the binary files that way (and/or some smarts), that'd be great!

scottdorman commented 8 years ago

I updated the pull request so it can work with just the path to the project file, the project file and path to the binary, or the project file and a configuration name. I also updated the command line section in the readme.

clairernovotny commented 8 years ago

Looks good -- I made a few code cleanups based on R# and pushed that back to the netstandard-pr branch. One final thing -- if the parameters are going to be projectfile=assemblyPath for the last grouping, can you please add an MSBuild mechanism to join the current @(NuSpecProjectFile) @(NuSpecLibContent) into those pairs? That's the last missing piece to support the current set off functionality.

clairernovotny commented 8 years ago

The reason, BTW, for those input arrays is also to support being overridden in the csproj file if the user has a more advanced scenario that needs to package multiple dlls into a single nuget package.

scottdorman commented 8 years ago

I'll take a look at adding that in the morning.

clairernovotny commented 8 years ago

I'm also ok if it's easier just to keep them as two separate parameters and zip them in code the right way.

I was also thinking of using the new System.Commandline stuff to do the argument parsing. I can add that to the backlog :)