From af78adb4fe661781dbc7c34957949c71d0f29261 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Wed, 3 Jan 2018 11:08:38 -0800 Subject: [PATCH 1/3] spec_helper: Restore case-insensitive matching in find_spdx The previous case-insensitive matching was removed in e5f46faa (test required spdx-ids against data from spdx, 2016-05-25, #418). That commit was designed [1] to allow case-sensitive matching as discussed in [2]. But while I'm in favor of case-sensitive keys in spdx_list, the case-sensitive match breaks script/check-approval which downcases its argument since it was added in 8e56bb83 (add script/check-approval, 2016-01-18, #318). There are more notes on SPDX's plans for case sensitivity in [3], so we should see a clearer policy there soon. I'm arguing for case-sensitive *display* with optional case-insensitive matching. I am optimistic that the SPDX will at least agree not to register short IDs that only differ by case, which is all we need to make this case-insensitive match safe here. [1]: https://github.com/github/choosealicense.com/pull/418#issuecomment-221404630 [2]: https://github.com/benbalter/licensee/issues/72 [3]: https://github.com/spdx/spdx-spec/issues/63 --- spec/spec_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index e66a9a1..c8c2a52 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -86,7 +86,7 @@ def spdx_ids end def find_spdx(license) - spdx_list.find { |name, _properties| name == license } + spdx_list.find { |name, _properties| name.casecmp(license).zero? } end def osi_approved_licenses From 84a7bbbf968406403bcd57d3ebd4973ca6989cc9 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Wed, 3 Jan 2018 11:17:19 -0800 Subject: [PATCH 2/3] spec_helper: Extract FSF approval from wking.github.io/fsf-api Ideally the FSF would be maintaining the API (or any API), but until someone can talk them into that I think we can save work by collaborating on the mock API. Using a JSON API also allows us to drop the Nokogiri dependency. The parens feel excessive, and I'm not familiar with Ruby, so they might be. However, removing the parens from the libre check resulted in: $ ./script/check-approval ISC ./script/check-approval:8:in `require_relative': /.../choosealicense.com/spec/spec_helper.rb:108: syntax error, unexpected tSTRING_BEG, expecting keyword_then or ';' or '\n' (SyntaxError) ...gs') && meta['tags'].include? 'libre' ... ^ /.../choosealicense.com/spec/spec_helper.rb:116: syntax error, unexpected keyword_end, expecting end-of-input from ./script/check-approval:8:in `
' --- Gemfile | 1 - spec/spec_helper.rb | 23 +++++++---------------- 2 files changed, 7 insertions(+), 17 deletions(-) diff --git a/Gemfile b/Gemfile index c92472d..6677662 100644 --- a/Gemfile +++ b/Gemfile @@ -17,7 +17,6 @@ end group :test do gem 'html-proofer', '~> 3.0' gem 'licensee' - gem 'nokogiri' gem 'rake' gem 'rspec' gem 'rubocop' diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index c8c2a52..2fb5fd8 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -4,7 +4,6 @@ require 'jekyll' require 'json' require 'licensee' require 'open-uri' -require 'nokogiri' module SpecHelper class << self @@ -102,23 +101,15 @@ end def fsf_approved_licenses SpecHelper.fsf_approved_licenses ||= begin - url = 'https://www.gnu.org/licenses/license-list.en.html' - doc = Nokogiri::HTML(open(url).read) - list = doc.css('.green dt') + url = 'https://wking.github.io/fsf-api/licenses-full.json' + object = JSON.parse(open(url).read) licenses = {} - list.each do |license| - a = license.css('a').find { |link| !link.text.nil? && !link.text.empty? && link.attr('id') } - next if a.nil? - id = a.attr('id').downcase - name = a.text.strip - licenses[id] = name + object.each_value do |meta| + next unless (meta.include? 'identifiers') && (meta['identifiers'].include? 'spdx') && (meta.include? 'tags') && (meta['tags'].include? 'libre') + meta['identifiers']['spdx'].each do |identifier| + licenses[identifier.downcase] = meta['name'] + end end - - # FSF approved the Clear BSD, but doesn't use its SPDX ID or Name - if licenses.keys.include? 'clearbsd' - licenses['bsd-3-clause-clear'] = licenses['clearbsd'] - end - licenses end end From 45369acf791426004f7f0f5da7346523fe279399 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Wed, 3 Jan 2018 10:46:57 -0800 Subject: [PATCH 3/3] check-approval: Fix missing license_ids variable Avoid: $ ./script/check-approval MIT ./script/check-approval:55:in `
': undefined local variable or method `license_ids' for main:Object (NameError) The license_ids line was added in e505eb8f (check if license is aleady a license, 2016-01-18, #318), but license_ids was removed from the helper in b99e7ab0 (replace 'id' variables with 'spdx_lcase' to minimize confusion, 2016-06-01, #424). This commit restores the old code locally, since this script is the only consumer. --- script/check-approval | 1 + 1 file changed, 1 insertion(+) diff --git a/script/check-approval b/script/check-approval index 6b491db..6451818 100755 --- a/script/check-approval +++ b/script/check-approval @@ -52,6 +52,7 @@ approvals.each do |approver, licenses| rows << ["#{approver} approved", licenses.include?(license)] end +license_ids = licenses.map { |l| l['id'] } current = license_ids.include?(license) rows << ['Current license', current]