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

Dependencies cache all available license files #64

Merged
merged 4 commits into from
Jul 10, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 29 additions & 22 deletions lib/licensed/dependency.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,24 +68,24 @@ def local_files
# Resets all local project and license information
def reset_license!
@project = nil
@matched_project_file = nil
@project_license = nil
self.delete("license")
self.text = nil
end

# Returns the Licensee::ProjectFile representing the matched_project_file
# or remote_license_file
def project_file
matched_project_file || remote_license_file
end

# Returns the Licensee::LicenseFile, Licensee::PackageManagerFile, or
# Licensee::ReadmeFile with a matched license, in that order or nil
# if no license file matched a known license
def matched_project_file
@matched_project_file ||= project.matched_files
.select { |f| f.license && !f.license.other? }
.first
# Finds and returns a license from the Licensee::FSProject for this dependency.
def project_license
@project_license ||= if project.license && !project.license.other?
# if Licensee has determined a project to have a specific license, use it!
project.license
elsif project.license_files.map(&:license).uniq.size > 1

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea @mlinksva and I were discussing that at #64 (comment).

TL;DR it's not quite the same due to project.license including additional sources over project.license_files

# if there are multiple license types found from license files,
# return 'other'
Licensee::License.find("other")
else
# check other project files if we haven't yet found a license type
project.licenses.reject(&:other?).first

Choose a reason for hiding this comment

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

If a project is dual licensed, it will pick the first? What if it's dual licensed as GPL and Apache, in that order? Perhaps we should look for a non-copyleft license first to avoid false positives when a project is dual licensed (e.g., "choose" to use Apache over GPL in that case).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was added to maintain legacy behavior where licensed previously chose the first non-other license found from project.matched_files. If it's a good idea to intentionally change that behavior, 👍

end
end

# Returns a Licensee::LicenseFile with the content of the license in the
Expand All @@ -96,21 +96,28 @@ def remote_license_file
end
Copy link
Member

Choose a reason for hiding this comment

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

Not part of this change, but just idly wondering why above two lines can't just be

@remote_license_file ||= Licensed.from_github(self["homepage"])

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 question! It's because ||= doesn't stop Licensed.from_github(self["homepage"]) from being called again when a previous call returned nil


# Regardless of the license detected, try to pull the license content
# from the local LICENSE, remote LICENSE, or the README, in that order
# from the local LICENSE-type files, remote LICENSE, or the README, in that order
def license_text
content_file = project.license_file || remote_license_file || project.readme_file
content_file.content if content_file
content_files = Array(project.license_files)
content_files << remote_license_file if content_files.empty? && remote_license_file
content_files << project.readme_file if content_files.empty? && project.readme_file
content_files.map(&:content).join("\n#{LICENSE_SEPARATOR}\n")
end

# Returns a string representing the project's license
# Note, this will be "other" if a license file was found but the license
# could not be identified and "none" if no license file was found at all
def license_key
if project_file && project_file.license
project_file.license.key
elsif project.license_file || remote_license_file
if project_license && !project_license.other?
# return a non-other license found from the Licensee
project_license.key
elsif remote_license_file && !remote_license_file.license.other?
# return a non-other license found from the remote source
remote_license_file.license.key
elsif project.license || remote_license_file
# if a license was otherwise found but couldn't be identified to a
# single license, return "other"
"other"
else
# no license found
"none"
end
end
Expand Down
25 changes: 25 additions & 0 deletions test/dependency_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,31 @@ def mkproject(&block)
end
end

it "extracts license text from multiple license files" do
mkproject do |dependency|
File.write "LICENSE", Licensee::License.find("mit").text
File.write "LICENSE.md", Licensee::License.find("bsd-3-clause").text

dependency.detect_license!

assert_equal 1, dependency.text.scan(/#{Regexp.escape(Licensed::License::LICENSE_SEPARATOR)}/).size
assert dependency.text.include?(Licensee::License.find("mit").text.strip)
assert dependency.text.include?(Licensee::License.find("bsd-3-clause").text.strip)
assert_equal "other", dependency["license"]
end
end

it "does not detect a license from package manager when multiple license files are given" do
mkproject do |dependency|
File.write "LICENSE", Licensee::License.find("mit").text
File.write "LICENSE.md", "See project.gemspec"
File.write "project.gemspec", "s.license = 'mit'"

dependency.detect_license!
assert_equal "other", dependency["license"]
end
end

it "always contains a license text section if there are legal notices" do
mkproject do |dependency|
File.write "AUTHORS", "authors"
Expand Down