Package LICENSE with each component#790
Conversation
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #790 +/- ##
==========================================
- Coverage 74.98% 74.85% -0.14%
==========================================
Files 215 220 +5
Lines 12609 12683 +74
==========================================
+ Hits 9455 9494 +39
- Misses 3154 3189 +35 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Pull Request Test Coverage Report for Build 5bc7253136e7040b8ef28195fbde1d5e4079acac-PR-790
💛 - Coveralls |
| for path in ${handle} | ||
| root_dir = dirname ${path} | ||
| if not is_empty ${root_dir} | ||
| if not is_path_exists ${root_dir}/LICENSE |
There was a problem hiding this comment.
suggestion (nb): should we check that it is the same license, too?
There was a problem hiding this comment.
something like if cmp -s ${root_dir}/LICENSE ${root_dir}/../LICENSE (don't know my duckscript)
There was a problem hiding this comment.
I tried to do this prior to pushing this up for the initial review, but I was unable to get it working in duckscript:
license_text = readfile ${license}
if not eq ${license_text} ${license_text}
echo "not working"
end
but it turns out that this works:
license_text = readfile ${license}
license_equal = eq ${license_text} ${license_text}
if not eq ${license_equal}
echo "not working"
end
Duckscript is not intuitive to me at all.
| end | ||
| ''' | ||
|
|
||
| [tasks.license-file-check] |
There was a problem hiding this comment.
This should be a part of the ci-task-tidy task in the toplevel Makefile
| root_dir = dirname ${path} | ||
| if not is_empty ${root_dir} | ||
| if not is_path_exists ${root_dir}/LICENSE | ||
| trigger_error "LICENSE file missing in ${root_dir}" |
There was a problem hiding this comment.
suggestion: mention something like "run cargo make copy-license to automatically copy the license to all necessary locations"
There was a problem hiding this comment.
is there any linter on the lower level .toml files themselves? maybe something to enforce consistency (the all have the same categories etc) would be useful.
There was a problem hiding this comment.
Not yet, but that's a good idea!
|
Non blocking: I really wish cargo would know how to package in files from @Manishearth do you agree? Could we file an issue against cargo? |
|
Yes, please do! I'm not sure how it should work but worth filing an issue. |
7b02ca4
|
Filed as rust-lang/cargo#9575 |
|
It looks like this is actually a problem when running |
|
@zbraniecki My take on the upstream bug is that the vendoring issue is not going to be easy to solve, and packaging the LICENSE file with each component is a viable workaround. What do you think? |
|
Agree. |
|
@dminor That's not clear to me at all; it seems like a minor bug in cargo vendor to me so far. I'd rather avoid doing this if we don't have to. |
I agree, I'd prefer to not do this either. Are you willing to fix the upstream bug? If so, I'm happy to close this again and move on with my life :) |
|
I don't have the time or the knowledge to fix the upstream bug; but I'm pushing on it. |
|
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
|
From conversation on slack, the plan is to go ahead with this workaround until the upstream cargo issue is fixed. |
This packages LICENSE with each component and adds commands to automatically copy LICENSE files to each component and a check to make sure LICENSE exists in each component.
Fixes #771.