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.

Sign up for GitHub

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

Jump to bottom

Add fixture with invalid encoding, update FSProject load_file to force UTF8 #420

Merged
merged 4 commits into from Apr 3, 2020
Merged

Add fixture with invalid encoding, update FSProject load_file to force UTF8 #420

merged 4 commits into from Apr 3, 2020

Conversation

jvonkluck
Copy link
Contributor

@jvonkluck jvonkluck commented Mar 25, 2020 •
edited by markdownify bot

I'm using the licensee gem as a part of the licensed gem for validating a number of closed source projects at my job. One of the projects in particular pulled in some npm dependencies which the author kindly included non-UTF8 characters in.

The current implementation includes some logic to clean up poorly encoded files in ProjectFiles::ProjectFile.initialize but the Project class attempts to retrieve license content from a readme match before instantiating a ProjectFile.

This felt like a minimally intrusive albeit slightly repetitive way to deal with poorly encoded input files.

I'm not positive that this is in keeping with the overall architecture but am hoping to provide a solution for a problem that I'd like to fix for my own usage.

View rendered spec/fixtures/readme-invalid-encoding/README.md

@welcome
Copy link

welcome bot commented Mar 25, 2020

Welcome! Congrats on your first pull request to Licensee. If you haven't already, please be sure to check out the contributing guidelines.

@jvonkluck
Copy link
Contributor Author

FWIW - the spec failure in travis
rspec ./spec/licensee/commands/detect_spec.rb:72 # detect command json returns the expected output
is occurring for me locally in master as well.

@jvonkluck
Copy link
Contributor Author

jvonkluck commented Mar 26, 2020 •
edited

Additional context - here's the stacktrace of the invocation / unhandled exception when using licensed:

...
    Caching @babel/traverse-7.9.0 (7.9.0)
    Caching @babel/types-7.6.3 (7.6.3)
    Caching @babel/types-7.9.0 (7.9.0)
    Caching @cnakazawa/watch (1.0.4)
Traceback (most recent call last):
	46: from /usr/local/bundle/bin/licensed:23:in `<main>'
	45: from /usr/local/bundle/bin/licensed:23:in `load'
	44: from /usr/local/bundle/gems/licensed-2.9.1/exe/licensed:5:in `<top (required)>'
	43: from /usr/local/bundle/gems/thor-0.20.3/lib/thor/base.rb:466:in `start'
	42: from /usr/local/bundle/gems/thor-0.20.3/lib/thor.rb:387:in `dispatch'
	41: from /usr/local/bundle/gems/thor-0.20.3/lib/thor/invocation.rb:126:in `invoke_command'
	40: from /usr/local/bundle/gems/thor-0.20.3/lib/thor/command.rb:27:in `run'
	39: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/cli.rb:14:in `cache'
	38: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/cli.rb:83:in `run'
	37: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/commands/command.rb:22:in `run'
	36: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/reporters/reporter.rb:61:in `report_run'
	35: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/commands/command.rb:27:in `block in run'
	34: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/commands/command.rb:27:in `map'
	33: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/commands/command.rb:27:in `block (2 levels) in run'
	32: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/commands/command.rb:56:in `run_app'
	31: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/reporters/cache_reporter.rb:12:in `report_app'
	30: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/reporters/reporter.rb:83:in `report_app'
	29: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/reporters/cache_reporter.rb:14:in `block in report_app'
	28: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/commands/command.rb:57:in `block in run_app'
	27: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/commands/command.rb:57:in `chdir'
	26: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/commands/command.rb:64:in `block (2 levels) in run_app'
	25: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/commands/command.rb:64:in `map'
	24: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/commands/command.rb:64:in `block (3 levels) in run_app'
	23: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/commands/cache.rb:26:in `run_source'
	22: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/commands/command.rb:81:in `run_source'
	21: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/reporters/cache_reporter.rb:26:in `report_source'
	20: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/reporters/reporter.rb:106:in `report_source'
	19: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/reporters/cache_reporter.rb:28:in `block in report_source'
	18: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/commands/command.rb:87:in `block in run_source'
	17: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/commands/command.rb:87:in `map'
	16: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/commands/command.rb:87:in `block (2 levels) in run_source'
	15: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/commands/command.rb:109:in `run_dependency'
	14: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/reporters/cache_reporter.rb:76:in `report_dependency'
	13: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/reporters/reporter.rb:128:in `report_dependency'
	12: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/reporters/cache_reporter.rb:77:in `block in report_dependency'
	11: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/commands/command.rb:119:in `block in run_dependency'
	10: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/commands/cache.rb:48:in `evaluate_dependency'
	 9: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/dependency.rb:60:in `record'
	 8: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/dependency.rb:136:in `license_metadata'
	 7: from /usr/local/bundle/gems/licensed-2.9.1/lib/licensed/dependency.rb:68:in `license_key'
	 6: from /usr/local/bundle/gems/licensee-9.13.1/lib/licensee/projects/project.rb:27:in `license'
	 5: from /usr/local/bundle/gems/licensee-9.13.1/lib/licensee/projects/project.rb:158:in `licenses_without_copyright'
	 4: from /usr/local/bundle/gems/licensee-9.13.1/lib/licensee/projects/project.rb:47:in `matched_files'
	 3: from /usr/local/bundle/gems/licensee-9.13.1/lib/licensee/projects/project.rb:151:in `project_files'
	 2: from /usr/local/bundle/gems/licensee-9.13.1/lib/licensee/projects/project.rb:79:in `readme_file'
	 1: from /usr/local/bundle/gems/licensee-9.13.1/lib/licensee/project_files/readme_file.rb:44:in `license_content'
/usr/local/bundle/gems/licensee-9.13.1/lib/licensee/project_files/readme_file.rb:44:in `match': invalid byte sequence in UTF-8 (ArgumentError)

mlinksva
mlinksva requested changes Apr 1, 2020
View reviewed changes
Copy link
Contributor

@mlinksva mlinksva left a comment

Choose a reason for hiding this comment

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

Small change and a question.

spec/fixtures/fixtures.yml Outdated Show resolved Hide resolved
lib/licensee/projects/fs_project.rb Outdated Show resolved Hide resolved
@mlinksva
Copy link
Contributor

mlinksva commented Apr 1, 2020

@jvonkluck the spec failure you mentioned at #420 (comment) was unrelated to this PR, now fixed. It's now passing except for rubocop complaints that do pertain to this PR.

@mlinksva mlinksva merged commit 5d6fbc6 into licensee:master Apr 3, 2020
@welcome
Copy link

welcome bot commented Apr 3, 2020

Congrats on getting your first pull request to Licensee merged! Without amazing humans like you submitting pull requests, we couldn’t run this project. You rock! 🎉

If you're interested in tackling another bug or feature, take a look at the open issues, especially those labeled help wanted.

@jvonkluck
Copy link
Contributor Author

@mlinksva Thank you very much for the quick turnaround. What's the process / schedule for getting a patch release published to Rubygems now that the fix has been merged in to master?

@jvonkluck jvonkluck deleted the force-utf8-encoding-fs-project-files branch April 3, 2020 16:45
@mlinksva
Copy link
Contributor

mlinksva commented Apr 3, 2020

@benbalter is the lead and does releases. AFAIK he does them as needed && when he has time. 😄

@jvonkluck
Copy link
Contributor Author

Ha - totally understandable. It's free time for all of us...

@jonabc jonabc mentioned this pull request Apr 28, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mlinksva mlinksva mlinksva approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@jvonkluck @mlinksva