Skip to content

Conversation

@GeorgRottensteiner
Copy link

Allow package lookup from a list of repositories

Allow package lookup from a list of repositories
@GeorgRottensteiner GeorgRottensteiner requested a review from a team as a code owner January 16, 2025 06:52
GeorgRottensteiner and others added 5 commits January 21, 2025 15:02
Signed-off-by: Georg Rottensteiner <georg@georg-rottensteiner.de>
Merge in VAS/cyclonedx-fork from feature/fix_nuget_zip_crash to master

* commit 'defa3ac1fa89ce0fe67f045573ef4d100883d058':
  Fix Codacy "issues" Signed-off-by: Georg Rottensteiner <georg@georg-rottensteiner.de>
  Added NugetConfigFileService, parsing nuget.config for package sources Allow package lookup from a list of repositories
@mtsfoni
Copy link
Member

mtsfoni commented Feb 9, 2025

Is this related to #904? Could you give me a little more context before I review?

Also you need to sign off your commits. Here is also a detailed guide how to retrospectively: https://github.com/CycloneDX/cyclonedx-dotnet/pull/921/checks?check_run_id=35933831961

@GeorgRottensteiner
Copy link
Author

Looks like it. The issue is that NuGet.Protocol returns a non-null Stream object even if there is no file at all.
One thing that I did is add a check for that (length == 0 )
Obviously that is not enough, since there is a reason the file is not there. That's why I opted to add nuget.config parsing. Not sure if this is the proper way to record several NuGet sources. NuGet keeps shuffling options and methods unfortunately.

Also as a hint: I did not add package source authentication, albeit the models allowing for such. I did more or less the bare minimum to get it working for my use case. If that method works that might be something that ought to be added.

Will look into that signing off asap!

Georg Rottensteiner and others added 2 commits February 9, 2025 19:15
Allow package lookup from a list of repositories
Modified commit message to include signoff:
Signed-off-by: Georg Rottensteiner <georg@georg-rottensteiner.de>
Signed-off-by: Georg Rottensteiner <georg@georg-rottensteiner.de>
@GeorgRottensteiner GeorgRottensteiner force-pushed the feature/fix_nuget_zip_crash branch from defa3ac to f8ac92e Compare February 9, 2025 18:16
@GeorgRottensteiner
Copy link
Author

Not sure if I did that correctly, I have a 50% miss rate with rebasing :)
The commits look like having the intended sign-off.
Never heard of that sign-off thingy before this PR.

Need that StringBuilder issue Codacy bemoans also rectified?

Signed-off-by: Georg Rottensteiner <georg@georg-rottensteiner.de>
@GeorgRottensteiner
Copy link
Author

I wanted to explain the other part I modified:
Since the projects I handle use more than one package source, I also replaced the single source with a list of sources. This requires gracefully handling the failure of finding a package on one source, and only abort if a package cannot be found on none of them.
In case of a package existing in several sources, first one wins. Basically the source order in nuget.config decides the priority. Which is IIRC also how NuGet handles this.

@mtsfoni
Copy link
Member

mtsfoni commented Feb 10, 2025

I think I understand and after skimming briefly the code looks good so far.

I'd like to set up an environment to recreate the problem and test your solution, as that is some work, it might take a little before I integrate this PR.
But thank you for your contribution already!

@mtsfoni mtsfoni self-assigned this Feb 10, 2025
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.

2 participants