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

Check for cached metadata version before comparing it #41

Merged
merged 1 commit into from
May 31, 2018

Conversation

jonabc
Copy link
Contributor

@jonabc jonabc commented May 30, 2018

Fixes an issue where a new dependency's license metadata was not being cached when version metadata for the dependency was not available.

This bug occurs as a result of #21. Previous to that change, licensed only considered reusing cached license data when the cached data existed. A file existence check was removed to optimize for using cached data for more than version checks.

github/linguist hit an edge case where their license information, which doesn't contain version metadata, began to view non-existent metadata as the same version as new metadata and skipped caching the new content.

This PR adds a check that the cached data contains non-nil version metadata before checking if the version metadata is current. Since we can't verify version information that doesn't exist, nil or empty version metadata will force rechecking and caching license content.

/cc @lildude FYI and review as the reporter of the bug

Copy link
Member

@lildude lildude left a comment

Choose a reason for hiding this comment

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

With a little tweaking of how Linguist calls licensed (to take into account the changes introduced in #33), I can confirm this resolves the problem from a Linguist point of view:

$ script/licensed --module /Users/lildude/github/linguist/vendor/grammars/m3
Caching licenes for linguist:
  grammar dependencies:
caching /Users/lildude/github/linguist/vendor/grammars/m3
    Caching m3 ()
License caching complete!
* linguist grammar dependencies: 1
$ ls vendor/licenses/grammar/m3.txt
vendor/licenses/grammar/m3.txt
$

@jonabc jonabc merged commit 3a6b5ce into master May 31, 2018
@jonabc jonabc deleted the nil-license-version-fixes branch May 31, 2018 15:33
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