fix(scancode): No ScanCode license texts in disclosure document #9622
Conversation
wkl3nk
force-pushed
the
wkl3nk/quick-fix-scancode-license-text-location
branch
from
303e82f to
7f58b15
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9622 +/- ##
============================================
- Coverage 68.02% 67.96% -0.06%
- Complexity 1285 1294 +9
============================================
Files 249 249
Lines 8828 8899 +71
Branches 921 944 +23
============================================
+ Hits 6005 6048 +43
- Misses 2433 2457 +24
- Partials 390 394 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
wkl3nk
force-pushed
the
wkl3nk/quick-fix-scancode-license-text-location
branch
from
7f58b15 to
e01d408
Compare
wkl3nk
force-pushed
the
wkl3nk/quick-fix-scancode-license-text-location
branch
2 times, most recently
from
d24b05e to
de1b915
Compare
wkl3nk
force-pushed
the
wkl3nk/quick-fix-scancode-license-text-location
branch
from
de1b915 to
dd37d32
Compare
Dockerfile
Outdated
| @@ -157,6 +157,11 @@ RUN ARCH=$(arch | sed s/aarch64/arm64/) \ | |||
| rm requirements.txt; \ | |||
| fi | |||
|
|
|||
| # Extract ScanCode license texts to a directory | |||
| RUN scancode-license-data --path=/opt/scancode-license-data \ | |||
| && find /opt/scancode-license-data -type f ! -name "*.LICENSE" -exec rm -f {} + \ | |||
There was a problem hiding this comment.
Nit: Please use the more speaking -not instead of ! (as the latter also has a special meaning in Bash).
There was a problem hiding this comment.
I had a second thought on it: If scancode-license-data is the "API" way to access the license texts, then I don't want to put implementation-specific knowledge into the Dockerfile like what files I expect to be there, and which ones to delete. Because that might change over time, even without notice.
There was a problem hiding this comment.
If scancode-license-data is the "API" way to access the license texts
Well, the scancode-license-data command was originally designed to dump the data for a static website. In that sense, we're just "misusing" it here, and it's by-design that it does more than what we need.
There was a problem hiding this comment.
Because that might change over time, even without notice.
True. But to guard against that, I was planning to make a follow-up change that runs ort requirements as part of the Docker image build, to fail if ORT cannot find any licenses. We already were doing that at some point, but apparently it got dropped during some Docker image build process refactoring.
Also the commit message title should not be the one of the bug getting fixed, but say what's being done: |
wkl3nk
force-pushed
the
wkl3nk/quick-fix-scancode-license-text-location
branch
from
dd37d32 to
24e19d0
Compare
| @@ -493,6 +499,9 @@ ENV GEM_HOME=/var/tmp/gem | |||
| ENV PATH=$PATH:$RBENV_ROOT/bin:$RBENV_ROOT/shims:$RBENV_ROOT/plugins/ruby-install/bin | |||
| COPY --from=ruby --chown=$USER:$USER $RBENV_ROOT $RBENV_ROOT | |||
|
|
|||
| # ScanCode license texts directory | |||
| COPY --from=scancode-license-data --chown=$USER:$USER /opt/scancode-license-data /opt/scancode-license-data | |||
There was a problem hiding this comment.
Why did you remove the previous removal of unneeded files? Can we maybe elegantly replace that with only copying /opt/scancode-license-data/*.LICENSE here?
There was a problem hiding this comment.
Ah, I missed your comment as you already resolved / collapsed the conversation. So, still, what do you think about just copying needed files here?
There was a problem hiding this comment.
Copying the needed stuff is more complicated than expected, so I went back deleting the files we don't need.
There was a problem hiding this comment.
Copying the needed stuff is more complicated than expected
Oh, interesting, mind sharing the details? Why does
COPY --from=scancode-license-data --chown=$USER:$USER /opt/scancode-license-data/*.LICENSE /opt/scancode-license-data
not work?
I went back deleting the files we don't need.
Unfortunately, that now seems to miss deleting the static directory, or?
There was a problem hiding this comment.
Ping @wkl3nk just in case you overlooked this comment.
Dump the ScanCode license texts to directory /opt/scancode-license-data when creating the reporter docker container. This directory is used by ORT as fallback option if the ScanCode license texts cannot be located by the existing heuristic look-up algorithm. See oss-review-toolkit/ort#9622. Signed-off-by: Wolfgang Klenk <[email protected]>
Dump the ScanCode license texts to directory /opt/scancode-license-data when creating the ORT docker container. Use this directory as fallback option if the ScanCode license texts cannot be located by the existing heuristic algorithm. Fixes oss-review-toolkit#8147. Signed-off-by: Wolfgang Klenk <[email protected]>
wkl3nk
force-pushed
the
wkl3nk/quick-fix-scancode-license-text-location
branch
from
24e19d0 to
d532222
Compare
Dump the ScanCode license texts to directory /opt/scancode-license-data when creating the ORT docker container. Use this directory as fallback option if the ScanCode license texts cannot be located by the existing heuristic algorithm.
Fixes #8147.