Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use same logic as dotnet restore/build for dependency resolution #42

Merged
merged 3 commits into from
Feb 7, 2020
Merged

Use same logic as dotnet restore/build for dependency resolution #42

merged 3 commits into from
Feb 7, 2020

Conversation

coderpatros
Copy link
Member

@coderpatros coderpatros commented Nov 26, 2019

This is going to be another large pull request, sorry @stevespringett :/

This will resolve issues #27 and #15. And is based on PRs #41 and #39

A few positive points of note:

  • individual package resolution should be a lot quicker, we're now using the local nuget package and nuspec cache, although this resolves all dependencies now, so it can actually take longer overall
  • correct BoM contents because this uses the same project.assets.json file produced by the dotnet restore command and consumed by the build process
  • version ranges are now handled due to the above point
  • custom/private project nuget sources now supported out of the box as above
  • improved nuspec and license expression parsing by using nuget libraries

But, this could result in some issues being raised.

For example...
We have one project that with the old project file reference and nuget resolution method had 7 dependencies. Now that it factors in everything it is reporting 179.

Part of this is because the standard libraries, like the Microsoft.* and System.* packages, weren't being included but they are now.

This seems like a massive change because of the line counts. But most of that is just test data. I've also added a test step timeout of 10 minutes. I had inadvertently introduced an async deadlock that was only evident when running on Windows or in the GitHub Linux agent.

@coderpatros coderpatros marked this pull request as ready for review December 13, 2019 07:14
@stevespringett
Copy link
Member

#65 was merged. Would it be possible to resolve the conflict?

@stevespringett
Copy link
Member

Also, when will this PR be ready to merge? Still a work in progress?

@coderpatros
Copy link
Member Author

@stevespringett the code itself is good to go. Just want to make sure the github actions work as expected.

@coderpatros
Copy link
Member Author

This includes #71

@coderpatros
Copy link
Member Author

@stevespringett all checks have passed so should be good to merge this now. Although I'll probably create another PR a bit later tonight that includes all the dependency bumps as well.

stevespringett added a commit that referenced this pull request Feb 7, 2020
Upgrade all dependencies, includes PRs #71 and #42
@stevespringett stevespringett merged commit 6191329 into CycloneDX:master Feb 7, 2020
@coderpatros coderpatros deleted the project-assets-json branch February 7, 2020 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants