Package LICENSE with each component #790
Conversation
dminor
requested review from
echeran,
filmil,
nciric,
sffc,
zbraniecki and
a team
as code owners
Codecov Report
@@ Coverage Diff @@
## main #790 +/- ##
==========================================
- Coverage 75.48% 75.34% -0.15%
==========================================
Files 194 193 -1
Lines 12297 12833 +536
==========================================
+ Hits 9283 9669 +386
- Misses 3014 3164 +150
Continue to review full report at Codecov.
|
Pull Request Test Coverage Report for Build 5bc7253136e7040b8ef28195fbde1d5e4079acac-PR-790
💛 - Coveralls |
| 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.
| ''' | ||
|
|
||
| [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
tools/scripts/tidy.toml
Outdated
| 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. |
dminor
dismissed stale reviews from zbraniecki, srl295, and Manishearth
via
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 |
dminor
removed
the
blocked
dminor
requested review from
aethanyc,
gregtatum,
iainireland,
makotokato,
nordzilla and
a team
as code owners
|
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.