NicolaDonelli / py4ai-core

MIT License
0 stars 0 forks source link

make install failing to parse pyproject.toml #29

Closed NicolaDonelli closed 1 year ago

NicolaDonelli commented 1 year ago

By checking action outputs here I realised that in the Check typing linting and formatting job we have this snippet

==Building package distribution==
python setup.py --quiet sdist
warning: no previously-included files matching '*.py[co]' found under directory '*'
warning: Check: missing required meta-data: name, url

warning: Check: missing meta-data: either (author and author_email) or (maintainer and maintainer_email) should be supplied

UPDATING UNKNOWN-0+untagged.1.gda10cd2.dirty/py4ai/core/_version.py

This is strange since the name of the package should be py4ai-core-0.0.1+11.g667d4c4.dirty.

I think this could be due to two different problems:

  1. the name of the package is missing since, for the setuptools version installed by default in the docker image, the sdist command of setuptools is unaware of pep 517/8,so preinstall/prebuild is a required step (as explained here): a possible solution to this point is to change Makefile upgrading setuptools (and pip) before building pre-deps:
    $(pre_deps_tag):
    @echo "==Installing pip-tools and black=="
    ${PYTHON} -m pip install --upgrade --quiet pip
    grep "^pip-tools\|^black"  requirements/requirements_dev.in | xargs ${PYTHON} -m pip install --quiet
    grep "^tomli\|^setuptools"  requirements/requirements.in | xargs ${PYTHON} -m pip install --upgrade --quiet
    touch $(pre_deps_tag)
  2. the version of the package is missing since the executor seems to be unable to retrieve the last tag from the repository. This is quite annoying and, in a Docker container, can be solved by copying the .git folder but I am not sure yet how to solve it in the workflows.
deusebio commented 1 year ago
  1. the version of the package is missing since the executor seems to be unable to retrieve the last tag from the repository. This is quite annoying and, in a Docker container, can be solved by copying the .git folder but I am not sure yet how to solve it in the workflows.

Uhm, copying the .git does NOT seem the way to go. Docker build should be git agnostic. I'd really rather avoid this.

NicolaDonelli commented 1 year ago

Copying .git in Docker container (or at least a part of it) is necessary since versioneer is based on it to compute semantic versions based on tags.

I am actually quite puzzled that this issue is present also in workflows since they should be aware of git history...

deusebio commented 1 year ago

To be honest we have to make a decision:

  1. Have a minimal docker image (.git repo is bringing all version control history, if one would ever place a big file that would unnecessarily increase the docker image size)
  2. Have the version right in the docker image

Between the two I'd argue it would be best to go for reducing the docker image size. Also, I don't think having the version right on the docker image is so critical. Every release could produce a docker image with the right version of the docker image.

Beside, I believe it would be easy to modify the versioneer file in order to put the business logic that, if a certain env variable is set, we could tag the package with that, instead of figuring that out using the VSC system. Not sure we should do that, but it is something we could think about. But definitely, I'd really AVOID copying the git repo in the docker image ONLY for having one string out