Fixed setup.py and setup.cfg

[KOLANICH]

No description provided.

[dgelessus]

LGTM in general. A couple of minor questions though:

[GreyCat]

Can you clarify what is the purpose of this PR? There's no description, and title is as ambiguous as it can get:

Fixed setup.py and setup.cfg

What is/was wrong with setup.py/setup.cfg so it required fixing?

[KOLANICH]

What is/was wrong with setup.py/setup.cfg so it required fixing?

1. Obsolete approaches were used. Explicitly reading setup.cfg was no longer needed, it is read and merged automatically even if something is set in setup().
2. pep517 is a uniform way to trigger building. Implemented its support.
3. extraction of version without importing. Useful when cwd is an another dir.

Also I wonder if it is acceptible to make kaitaistruct a dir, so we can capture the version from a git tag and save it into version.py using setuptools_scm.

[higherorderfunctor]

What is/was wrong with setup.py/setup.cfg so it required fixing?

For me, this fixed being able to install with poetry.

poetry add git+https://github.com/kaitai-io/kaitai_struct_python_runtime.git

[ParseVersionError]
Unable to parse "attr: kaitaistruct.__version__".

I did have to rename the branch though since it is named the same as a file in the repo.

poetry add git+https://github.com/KOLANICH/kaitai_struct_python_runtime.git#setup.cfg

[CalledProcessError]
Command '['git', '--git-dir', '/tmp/pypoetry-git-kaitai_struct_python_runtime5m5xquv5/.git', '--work-tree', '/tmp/pypoetry-git-kaitai_struct_python_runtime5m5xquv5', 'checkout', 'setup.cfg']' returned non-zero exit s
tatus 128.

Clone of this PR with a different branch name.

poetry add git+https://github.com/higherorderfunctor/kaitai_struct_python_runtime.git#setup_fix

Updating dependencies
Resolving dependencies... (4.0s)

Writing lock file


Package operations: 1 install, 0 updates, 0 removals

  - Installing kaitaistruct (0.9 1d01b14)

[KOLANICH]

For me, this fixed being able to install with poetry.

please don't use poetry, it is destroying the ecosystem because it is harmful because it is wasteful, but matches well with the current fashionable cargo cult :( It is very sad.

[dgelessus]

@higherorderfunctor Although I don't agree with @KOLANICH's opinion of Poetry, if it fails to parse a setup.cfg with version = attr: ... in it, that's a bug in Poetry and not in the setup.cfg. (I'm actually surprised that Poetry accepts the new setup.cfg from this PR, considering that it still uses the attr: ... feature. Most likely it's interpreting the version as literally attr: __version__, as it looks like Poetry's code for parsing setup.cfg never handles attr: ... anywhere.)

I did have to rename the branch though since it is named the same as a file in the repo.

That is also a bug in Poetry - it's perfectly legal to have a Git branch name that is identical to the name of a file in that branch.

I'd encourage you to report both of these bugs on the Poetry issue tracker: https://github.com/python-poetry/poetry/issues

[higherorderfunctor]

I did have to rename the branch though since it is named the same as a file in the repo.

That is also a bug in Poetry - it's perfectly legal to have a Git branch name that is identical to the name of a file in that branch.

I believe you have to add -- to the checkout command to disambiguate file vs branch. Poetry isn't doing this and as far as I know there is no way to inject this (I would agree this is a bug).

https://stackoverflow.com/a/25322508

[dgelessus]

Well, this PR now does something completely different than before... This isn't "fixing" setup.py and setup.cfg, it's replacing them entirely with a pyproject.toml-based configuration.

Unfortunately, at this point, this is a complete non-starter for us. According to the setuptools documentation about pyproject.toml configuration:

✏️ Note

New in 61.0.0 (experimental)

⚠️ Warning

Support for declaring project metadata or configuring setuptools via pyproject.toml files is still experimental and might change (or be removed) in future releases.

We shouldn't rely on an experimental unstable feature that's not even a month old, unless there's a very good reason for it, and I'm not seeing that here - the current setup.py/setup.cfg work just fine.

Regardless of whether the feature is stable or not, requiring a new setuptools version is also a problem for Python version compatibility. setuptools 61.0.0 requires at least Python 3.7 (59.7.0 dropped support for older Python versions). Kaitai Struct still supports Python 3.4 and 2.7, so we can't require anything newer than setuptools 43.0.0 (from late 2019), the last version to support Python 3.4.

[KOLANICH]

(or be removed) in future releases.

feels like bullshit. PEP 621 is accepted, had been long a goal for setuptools and now it is implemented, there is no way back. If you dislike relying on setuptools experimental feature (it contains the bug I have mentioned, that's why version is not dynamic now), I can add 5 lines (2 to enable flit_core as a build backend, 1 a section header, 1 specifying the source file for flit and 1 empty line) and comment out the 2 lines enabling setuptools as a build backend, and the stuff will be built with flit_core, the project section stays the same.

Regardless of whether the feature is stable or not, requiring a new setuptools version is also a problem for Python version compatibility. setuptools 61.0.0 requires at least Python 3.7 (59.7.0 dropped support for older Python versions). Kaitai Struct still supports Python 3.4 and 2.7, so we can't require anything newer than setuptools 43.0.0 (from late 2019), the last version to support Python 3.4.

I wonder if we can backport the latest setuptools to 3.4. Usually python version requirement is raised because of often unjustified (to the point of it being a cargo cult) use of so called f-strings and typing hints for variables within function bodies. I guess it can be possible to write an AST transformer undoing it.

[KOLANICH]

Also requires-python specifies the version of a package to run the code on, not the one to build a wheel on.

[generalmimon]

@KOLANICH:

If you dislike relying on setuptools experimental feature (it contains the bug I have mentioned, that's why version is not dynamic now), I can add 5 lines (...), and the stuff will be built with flit_core, the project section stays the same.

I wonder if we can backport the latest setuptools to 3.4. (...) I guess it can be possible to write an AST transformer undoing it.

But why - what would be the point? Why not just use something established and stable? It makes no sense to me.

[KOLANICH]

1. It looks more tidy. I'm a mad perfectionist, you know.
2. It is more standardized and has more tools around it. And will likely have even more tools thanks to network effects.
3. setup.cfg is planned to be dropped in indefinite future.

[KOLANICH]

Anyway, I revert back to setup.cfg. The metadata can be moved into pyproject.toml in an another PR.

[dgelessus]

These changes all make sense IMO. Just one suggestion:

[KOLANICH]

Why don't you just use the Commit suggestion button that GitHub offers?

Most of JS-based functionality of GitHub is broken for me by now.

[dgelessus]

Thanks!