scylladb / scylla-tools-java

Apache Cassandra, supplying tools for Scylla
Apache License 2.0
53 stars 85 forks source link

install-dependencies.sh: s/python/python3/ #389

Closed tchaikov closed 7 months ago

tchaikov commented 7 months ago

python is EOL. and the scripts in this repo actually are using python3 instead python:

$ find . -name '*.py' -exec grep '#!' {} \;

also, "python" as a package name is not available in debian and and some of its derivative distros, see https://packages.debian.org/search?keywords=python&searchon=names&suite=stable&section=all and https://packages.ubuntu.com/search?suite=jammy&section=all&arch=any&keywords=python3&searchon=names

but python3, or better off python3-minimal is. see https://packages.debian.org/search?keywords=python3-minimal&searchon=names&suite=stable&section=all and https://packages.ubuntu.com/search?suite=jammy&section=all&arch=any&keywords=python3-minimal&searchon=names

ubuntu/jammy is picked, because it is currently the ubuntu-latest os provided by github workflow runners.

so, in this change, let's replace "python" with "python3", so that we can run install-dependencies.sh without error like:

Package python is not available, but is referred to by another package.
This may mean that the package is missing, has been obsoleted, or
is only available from another source
However the following packages replace it:
  2to3 python2-minimal python2 dh-python python-is-python3

E: Package 'python' has no installation candidate
tchaikov commented 7 months ago

@scylladb/scylla-maint hi maintainers, could you help review this change? i am trying to add a workflow to scylladb using github action, which failed like https://github.com/tchaikov/scylladb/actions/runs/8778841191/job/24085931123 .

would be great if we can use github's ubuntu-latest runner to perform some static analysis.

tchaikov commented 7 months ago

/cc @raphaelsc

denesb commented 7 months ago

I wonder if we need to regenerate the toolchain image for this PR. Does this change make a difference on fedora? I guess we should regenerate it anyway just to really test the change.

tchaikov commented 7 months ago

I wonder if we need to regenerate the toolchain image for this PR.

yeah, to be on the safe side, we should.

Does this change make a difference on fedora?

yes. as i am changing the feodra branch as well. but i don't have to. i did it just for the sake of completeness. what i care is but the ubuntu runner.

I guess we should regenerate it anyway just to really test the change.

if it's complicated. or does not worth the efforts. i have some workarounds:

  1. only change the ubuntu/debian branch
  2. edit the install-dependencies.sh in scylladb in the workflow, so that it does not call these java tools' install-dependencies.sh (actually i am working in this direction)
  3. wait until we drop this submodule (it will take longer, as we are still packaging nodetool, and we will be using cassandra-stress, in a relatively long run)
denesb commented 7 months ago

if it's complicated. or does not worth the efforts. i have some workarounds:

It is not complicated at all. I just execute a command and a few hours later we have the new toolchain. The hardest part is just remembering that one started a regenerate job half a day later.

tchaikov commented 7 months ago

i see. thank you Botond!

denesb commented 7 months ago

Actually, it is a bit more complicated because I just noticed that this is the java-tools submodule. So it needs some coordination.

mykaul commented 7 months ago

Where do we use Python in scylla-tools-java ? (note that CQLSH has moved to its own repo long time ago)

tchaikov commented 7 months ago

Where do we use Python in scylla-tools-java ? (note that CQLSH has moved to its own repo long time ago)

$ cd ~/dev/scylladb/tools/java/
$ find . -name '*.py'
dist/debian/debian_files_gen.py
doc/scripts/convert_yaml_to_adoc.py
doc/scripts/gen-nodetool-docs.py
scripts/create-relocatable-package.py

@mykaul hi Yaniv, we are just using python to build the reloc package and debian package. for the sake of self-containness, i think it's fair to include python (should be python3!) in the install-dependencies.sh in this repo.

mykaul commented 7 months ago

@mykaul hi Yaniv, we are just using python to build the reloc package and debian package. for the sake of self-containness, i think it's fair to include python (should be python3!) in the install-dependencies.sh in this repo.

I don't think build-time deps should be included - especially not if it's 'just' for packaging. Moreover - two scripts are for docs only - which never/rarely change, one is for Debian package (which is just odd? Why don't we have the package metadata?) and the relocatable is the only one truly needed, no?

denesb commented 7 months ago

Started regenerating the toolchain.

denesb commented 7 months ago

I don't think build-time deps should be included - especially not if it's 'just' for packaging. Moreover - two scripts are for docs only - which never/rarely change, one is for Debian package (which is just odd? Why don't we have the package metadata?) and the relocatable is the only one truly needed, no?

I think it is very much expected by anybody executing ./install-dependencies.sh to get all dependencies, build-time or run-time. BTW scylladb.git also pulls in python3 already, so dropping it from here will not change anything.

mykaul commented 7 months ago

I don't think build-time deps should be included - especially not if it's 'just' for packaging. Moreover - two scripts are for docs only - which never/rarely change, one is for Debian package (which is just odd? Why don't we have the package metadata?) and the relocatable is the only one truly needed, no?

I think it is very much expected by anybody executing ./install-dependencies.sh to get all dependencies, build-time or run-time. BTW scylladb.git also pulls in python3 already, so dropping it from here will not change anything.

OK, was hoping to slowly reduce our deps all around. Specifically, get rid of Python in the (future) Scylla-only container.

denesb commented 7 months ago

Started regenerating the toolchain.

PR with regenerated toolchain: https://github.com/scylladb/scylladb/pull/18394

Once CI passes, I will merge this and rebase that PR to include the submodule update.

denesb commented 7 months ago

submodule update PR: https://github.com/scylladb/scylladb/pull/18394

denesb commented 7 months ago

Removed from master, because we currently cannot generate new toolchains, due some changes around the PGO compilers. @tchaikov please re-submit this PR and I will merge once toolchains can be generated again.

tchaikov commented 7 months ago

@denesb thank you for your efforts. i recreated this change at #391