qiling icon indicating copy to clipboard operation
qiling copied to clipboard

Migrate to PEP 518

Open nullableVoidPtr opened this issue 2 years ago • 8 comments

Checklist

Which kind of PR do you create?

  • [x] This PR only contains minor fixes.
  • [ ] This PR contains major feature update.
  • [ ] This PR introduces a new function/api for Qiling Framework.

Coding convention?

  • [ ] The new code conforms to Qiling Framework naming convention.
  • [ ] The imports are arranged properly.
  • [ ] Essential comments are added.
  • [ ] The reference of the new code is pointed out.

Extra tests?

  • [x] No extra tests are needed for this PR.
  • [ ] I have added enough tests for this PR.
  • [ ] Tests will be added after some discussion and review.

Changelog?

  • [ ] This PR doesn't need to update Changelog.
  • [x] Changelog will be updated after some proper review.
  • [ ] Changelog has been updated in my PR.

Target branch?

  • [x] The target branch is dev branch.

One last thing


This PR makes the qiling package removes the setup.py script and replaces it with the declarative setup.cfg file, and ensures that it is compatible with the new pyproject.toml meta build system specified in PEP 518, as discussed in #1109.

Most of the package metadata including dependencies are included in anticipation of PEP 621/631 support in setuptools (see https://github.com/pypa/setuptools/issues/1688 and https://github.com/pypa/setuptools/pull/2970). Alternatively, Qiling can use https://github.com/pypa/flit) to rely entirely on the one TOML file, and I believe pip would work just the same.

nullableVoidPtr avatar Mar 09 '22 14:03 nullableVoidPtr

Erm, now we need to main two dependency?

xwings avatar Mar 23 '22 09:03 xwings

Ah, sorry if I didn't make it clear.

The list of runtime dependencies is the same, but it is duplicated twice to conform to a finalized PEP, which would eventually be supported in setuptools.

I brought up filt as an optional alternative, which maintainers could choose to use, but for now, in order to make this PR work with the current setuptools version (what everyone uses), I have specified dependencies in both the standard project TOML, and the declarative setuptools config file. This would get rid of the deprecated setup.py process.

There should be little/no change to how contributors develop.

nullableVoidPtr avatar Mar 24 '22 06:03 nullableVoidPtr

@elicn what do you think ?

xwings avatar Mar 31 '22 09:03 xwings

I think that keeping up to date is always a good idea; see my full feddback at #1109. Since I don't deal with the release and distribution processes I don't know whether this is going to add more burden or not. I guess it is for you to consider and decide.

elicn avatar Mar 31 '22 10:03 elicn

Ah, sorry if I didn't make it clear.

The list of runtime dependencies is the same, but it is duplicated twice to conform to a finalized PEP, which would eventually be supported in setuptools.

I brought up filt as an optional alternative, which maintainers could choose to use, but for now, in order to make this PR work with the current setuptools version (what everyone uses), I have specified dependencies in both the standard project TOML, and the declarative setuptools config file. This would get rid of the deprecated setup.py process.

There should be little/no change to how contributors develop.

I still dont understand where they are two files. one is TOML and one setuptools ?

xwings avatar Apr 16 '22 13:04 xwings

I still dont understand where they are two files. one is TOML and one setuptools ?

TOML (the PEP standard) is preferred as it would allow support for other build systems. Initially, when I started this PR, setuptools did not support TOML, just it's own setup.cfg, but now it does experimentally (see: pypa/setuptools#3214). I included both files at the time of writing expecting that it would be supported soon.

So we have several options:

  • Choose to keep one of the two files:
    • pyproject.toml which is a new standard only just experimentally supported in very recent versions of setuptools>=61.0.0, or
    • setup.cfg which works for only setuptools, but is considered stable and the successor to the legacy setup.py method
  • Keep both files, allowing Qiling developers to safely package and develop on setuptools, but also allows use of alternative systems like poetry or flit
  • Close this PR and maintain the legacy setup.py.

Sorry if it still doesn't make sense; Python building and packaging is a bit of a mess 😅

nullableVoidPtr avatar Apr 30 '22 06:04 nullableVoidPtr

I still dont understand where they are two files. one is TOML and one setuptools ?

TOML (the PEP standard) is preferred as it would allow support for other build systems. Initially, when I started this PR, setuptools did not support TOML, just it's own setup.cfg, but now it does experimentally (see: pypa/setuptools#3214). I included both files at the time of writing expecting that it would be supported soon.

So we have several options:

  • Choose to keep one of the two files:

    • pyproject.toml which is a new standard only just experimentally supported in very recent versions of setuptools>=61.0.0, or
    • setup.cfg which works for only setuptools, but is considered stable and the successor to the legacy setup.py method
  • Keep both files, allowing Qiling developers to safely package and develop on setuptools, but also allows use of alternative systems like poetry or flit

  • Close this PR and maintain the legacy setup.py.

Sorry if it still doesn't make sense; Python building and packaging is a bit of a mess 😅

It's always a big mess but funny things are that everything "just" happens to work.

Personally, I prefer poetry and it looks good to me.

wtdcode avatar Apr 30 '22 09:04 wtdcode

It appears that a zip distribution is currently used as a dependency in the develop branch; I'll need to look into how to resolve this with PEP 518.

nullableVoidPtr avatar May 01 '22 04:05 nullableVoidPtr

@wtdcode @kabeor @elicn there is some update and what do u guys think.

xwings avatar Oct 06 '22 03:10 xwings

@nullableVoidPtr there is a conflict to solve :)

xwings avatar Oct 06 '22 03:10 xwings

It definitely make package info more readable, I think it should be merged once the conflict is solved.

kabeor avatar Oct 06 '22 05:10 kabeor

Hi all,

Totally forgot about this PR, life got in the way. @kabeor @xwings, am I still to maintain both the TOML and the old setup.py, or should Qiling use pyproject.toml only?

nullableVoidPtr avatar Oct 07 '22 04:10 nullableVoidPtr

I've taken the liberty to remove most of the setup.py code as the TOML code is no longer experimental and should be stable.

Also, I've noticed that some variables, like version, can be marked as dynamic and depend on the contents of a file; this may be useful in conjunction with something like setuptools-scm, but I'm not sure how wanted something like that would be.

nullableVoidPtr avatar Oct 20 '22 16:10 nullableVoidPtr

One question,

we save and read the version number from setup.py

This will be a problem if we remove from setup.py

xwings avatar Oct 21 '22 03:10 xwings

we save and read the version number from setup.py Ah right, what with the dev branch tagging. I currently don't see any code in-tree, and it should be fairly easy to automatically edit a TOML file, but I understand if it's a part of others' workflow and if it may be difficult to change.

Okay, so one option is to have project.dynamic = ['version'], and further something like version = {attr = "setup.VERSION"}. Probably not the best way to go about it, but it might work.

If the need for the one version is not needed in setup.py, then this could be just __version__ in qiling/__init__py, removing the need for the importlib dependency there, though it's up to you.

FWIW, this diff works wonders:

diff --git a/pyproject.toml b/pyproject.toml
index 636daf17..023d426f 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -1,7 +1,7 @@
 [project]
 name = "qiling"
-version = "1.4.5-dev"
 description = "Qiling is an advanced binary emulation framework that cross-platform-architecture"
+dynamic = ["version"]
 readme = "README.md"
 requires-python = ">=3.8"
 license = {text = "GPL-2.0-only"}
@@ -69,7 +69,7 @@ changelog = "https://github.com/qilingframework/qiling/blob/master/ChangeLog"
 qltool = "qltool:main"
 
 [build-system]
-requires = ["setuptools>=61.0.0", "wheel"]
+requires = ["setuptools>=61.0.0", "setuptools_scm[toml]>=6.2", "wheel"]
 build-backend = "setuptools.build_meta"
 
 [tool.setuptools.package-data]
@@ -78,3 +78,4 @@ qiling = ["profiles/*.ql"]
 "qiling.os.uefi" = ["guids.csv"]
 "qiling.arch.evm.analysis" = ["signatures.json"]
 
+[tool.setuptools_scm]

On my machine, this builds version 1.4.5.dev279+g0dcf436c.d20221021 on my machine, but may need tweaking to fit Qiling's purposes

nullableVoidPtr avatar Oct 21 '22 04:10 nullableVoidPtr

I guess there is a problem between qiling and qltools. that is the reason why importlib is there.

and having version number in 2 or 3 files is not a good way to do it.

@elicn what do you think ?

xwings avatar Oct 21 '22 05:10 xwings

I like @nullableVoidPtr's approach and would happily drop the importlib thing from __init__.py.

However, I didn't understand what is the problem with Qiling / qltool. Can't they both rely on what's in __init__.py ?

elicn avatar Oct 21 '22 14:10 elicn

I dont remember why it all move to importlib, it was in init.py all this while until someone made some changes to it ?

maybe @wtdcode knows ?

xwings avatar Oct 25 '22 08:10 xwings

The importlib is introduced by @nullableVoidPtr .

I moved the version to a single file and get it by manually exec because previously Qiling setup.py will assume Qiling is already installed and retrieve Qiling version by import, which violates some setup.py assumptions.

wtdcode avatar Oct 25 '22 08:10 wtdcode

@wtdcode so, this PR is good to go ? some still need some fix ?

xwings avatar Oct 25 '22 08:10 xwings

I will again suggest @nullableVoidPtr testing the latest dev branch on the test PyPI.

Also, I doubt if our release CI still works after this change, which should be checked on test PyPI, too. Note the release steps are skipped on non-tag commits.

wtdcode avatar Oct 25 '22 08:10 wtdcode