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.

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

Dockerfile uses removed --reindex-licenses flag #3155

Closed
s01ipsist opened this issue Nov 22, 2022 · 10 comments · Fixed by #3159
Closed

Dockerfile uses removed --reindex-licenses flag #3155

s01ipsist opened this issue Nov 22, 2022 · 10 comments · Fixed by #3159
Labels

Comments

@s01ipsist
Copy link

s01ipsist commented Nov 22, 2022

Description

The Dockerfile uses the --reindex-licenses flag
https://github.com/nexB/scancode-toolkit/blob/develop/Dockerfile#L42
which was recently removed
5361052#diff-fad3fa964c566642a9a64df45efd8c9daeb435e1c8791ac1f64d2881b299d753L165
in this PR
#2979

so the Docker image no longer builds.

How To Reproduce

Follow install instructions on
https://scancode-toolkit.readthedocs.io/en/latest/getting-started/install.html#installation-via-docker

git clone https://github.com/nexB/scancode-toolkit
cd scancode-toolkit
docker build --tag scancode-toolkit --tag scancode-toolkit:$(git describe --tags) .

Result

 => ERROR [5/5] RUN ./scancode --reindex-licenses                        134.4s
------
 > [5/5] RUN ./scancode --reindex-licenses:
#9 0.370 * Configuring ScanCode for first use...
#9 133.8 Usage: scancode [OPTIONS] <OUTPUT FORMAT OPTION(s)> <input>...
#9 133.8 Try the 'scancode --help' option for help on options and arguments.
#9 133.8
#9 133.8 Error: No such option: --reindex-licenses (Possible options: --is-license-text, --license, --unknown-licenses)
------
executor failed running [/bin/sh -c ./scancode --reindex-licenses]: exit code: 2

System configuration

MacOS 13.0.1

@s01ipsist s01ipsist added the bug label Nov 22, 2022
@abhi-kr-2100
Copy link
Contributor

abhi-kr-2100 commented Nov 23, 2022

I would like to work on this. I've successfully built scancode-toolkit from source and ran it.

I plan to solve this issue as follows:

  • Try to understand how indexes are being rebuilt in the newer versions as --reindex-licenses has been removed.
  • Use the newer method instead of --reindex-licenses.

Does this sound good?

@s01ipsist
Copy link
Author

Sounds reasonable to me. 👍

@abhi-kr-2100
Copy link
Contributor

The newer way to reindex is the scancode-reindex-licenses command defined in https://github.com/nexB/scancode-toolkit/blob/develop/src/licensedcode/reindex.py#L15. It's installed as a separate executable.

The scancode bash script simply runs scancode (the Python script, not the bash script), but it should now be able to scancode-reindex-licenses. I think, a good solution would be to create another bash script scancode-reindex-licenses based on the original scancode bash script to run the scancode-reindex-licenses subcommand.

The dockerfile can be modified to use this new script in place of scancode --reindex-licenses.

Am I clear to do this?

@pombredanne
Copy link
Contributor

@abhi-kr-2100 re:

Am I clear to do this?
sure, thank you and you do not need to ask for permission!

but there is no need to create a bash script for scancode-reindex-licenses.... it can just be called directly either as scancode-reindex-licenses in an activated virtualenv, or as venv/bin/scancode-reindex-licenses otherwise. This is an exceptional operation and should therefore not need extra scripts

@pombredanne
Copy link
Contributor

Note also that it would be useful to have a test step in the CI that tests the creation and minimal run of the Dockerfile to avoid any future regression.

@abhi-kr-2100
Copy link
Contributor

@pombredanne @s01ipsist I used ./configure and venv/bin/scancode-reindex-licenses to do what ./scancode --reindex-licenses was doing earlier.

I don't know much about CI, but if you could provide some guidance, I would like to work on that as well. Will this be accomplished by a test run of docker in the CI?

@pombredanne
Copy link
Contributor

pombredanne commented Nov 23, 2022

Thanks... You can see https://github.com/nexB/scancode-toolkit/blob/a410f6ebb93a9b22343d7737897b61f3cd2a6ac3/azure-pipelines.yml for inspiration
You can use something such as https://stackoverflow.com/a/62871607 (See also reference doc at https://learn.microsoft.com/en-us/azure/devops/pipelines/tasks/reference/docker-v2?view=azure-pipelines&tabs=yaml )

The steps should be IMHO:

  1. build a docker image from a Dockerfile
  2. run the image with a sample, super simple scan, just validating that this did not fail.

@abhi-kr-2100
Copy link
Contributor

@pombredanne I've added the following to the azure-pipelines.yml just under the top-level jobs: key:

################################################################################
# Tests building with docker
################################################################################


- job: build_with_docker
  steps:
    - task: Docker@2
      displayName: Check that building using Docker succeeds
      inputs:
        command: 'build'
        tags: |
          'scancode-toolkit'
          'scancode-toolkit:$(Build.BuildId)'

I've never used Azure Pipelines, but according to what I could find on the internet, it's not possible to verify if an azure-pipelines.yml file a valid locally. Can you verify if the above is enough?

@pombredanne
Copy link
Contributor

@abhi-kr-2100 I would prefer some explicit reference to a Dockerfile .... and I cannot verify this either short of running it, so this needs to be in the PR
You could also have something that effectively runs a docker build instead of using a pre-configured task.
In all cases, you need to run the thing once built to validate that a scan can run. Check that building using Docker succeeds is not enough.

@abhi-kr-2100
Copy link
Contributor

@pombredanne I've added an explicit reference to the Dockerfile, and a script to run docker scan at the end. I only know the basics of Docker and almost nothing about Azure Pipelines, so please let me know everything I'm doing wrong here. Have a look at the latest PR #3159.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants