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

Find packages from Nuget fallback folders if possible #366

Merged
merged 6 commits into from
Jun 11, 2021

Conversation

jonabc
Copy link
Contributor

@jonabc jonabc commented May 31, 2021

closes #352

This gives an alternate solution to #273. I opted to start a new branch because that PR/branch had become stale and it was a bit quicker to build on the ideas rather than building directly on the code.

I've given attribution to @paveliak in the commit comment, and here as well I guess 😄 . @paveliak if there's another way you'd like me to represent your input in these changes please let me know 🙏 !

Some other changes that came along as I better understood the json file format and the code here:

  1. raising a Licensed::Sources::Source::Error error if the json file can't be parsed. This is a special error class that signals a source enumerator-wide error.
  2. setting a custom error message on the dependency if a path isn't found. a default error is set if a path is given but doesn't exist on disk, however in this case the path would be nil, which doesn't work with the default scenario.
  3. I refactored the code around finding reference keys to (I hope) simplify the code readability just a little bit. I'm using a Set to enforce uniqueness, though now the uniqueness is enforced for reference_key values and not computed id values.

+ raise error if json can't be parsed
+ set dependency error if path isn't found

based on solution by paveliak from
#273
@jonabc jonabc requested a review from zarenner May 31, 2021 04:01
@jonabc
Copy link
Contributor Author

jonabc commented Jun 10, 2021

@zarenner 👋 can you review this PR? I'm pretty confident it should be ok, but the sanity check on nuget usage and patterns as well as the extra set of eyes would be appreciated!

id = "#{name}-#{version}"

path = full_dependency_path(reference_key)
error = "Package #{id} not found at any project package folder" if path.nil?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't path also be nil if for some reason it's missing under libraries -> reference_key -> path? Not a big deal but the error will be slightly incorrect.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(feel free to ignore, I don't think this is important enough to differentiate on, but perhaps you want to clarify the message)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 good catch

@zarenner
Copy link
Contributor

lgtm!

@jonabc jonabc merged commit 38179a2 into master Jun 11, 2021
@jonabc jonabc deleted the nuget-fallback-folders branch June 11, 2021 22:04
jonabc added a commit that referenced this pull request Jun 16, 2021
## 3.1.0

2021-06-16

### Added

- Licensed supports Swift/Swift package manager as a dependency source (:tada: @mattt #363)'

### Changed

- The `source_path` configuration property accepts arrays of inclusion and exclusion glob patterns (#368)
- The Nuget source now uses configured fallback folders to find dependencies that are not in found in the project folder (#366)
- The Nuget source supports a configurable property for the path from the project source path to the project's `obj` folder (#365)

### Fixed
- The Go source's checks for local packages will correctly find paths in case-insensitive file systems (#370)
- The Bundler source will no longer unnecessarily reset the local Bundler environment configuration (#372)
@jonabc jonabc mentioned this pull request Jun 16, 2021
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.

NuGet Source should search in fallbackFolders if package is not found in packagesPath
2 participants