p4lang / ptf

Packet Test Framework
Apache License 2.0
144 stars 99 forks source link

Fix setup.cfg format issue. Remove empty requirements.txt. #193

Closed liushilongbuaa closed 1 year ago

onf-cla-manager[bot] commented 1 year ago

Hi @liushilongbuaa, this is the ONF bot 🤖 I'm glad you want to contribute to our projects! However, before accepting your contribution, we need to ask you to sign a Contributor License Agreement (CLA). You can do it online, it will take only a few minutes:

✒️ 👉 https://cla.opennetworking.org

After signing, make sure to add your Github user ID liushilongbuaa to the agreement.

For more information or help:" https://wiki.opennetworking.org/x/BgCUI

liushilongbuaa commented 1 year ago

The current setup.cfg is correct. However it requires an up-to-date build chain, as support was added about a year go to setuptools: pypa/setuptools#3253

Given that requirements.txt is currently empty, I see no harm in removing it and removing the install_requires directive in setup.cfg.

However, please also update the README.md file to remove references to requirements.txt.

I met errors after PR184 merged.

# git log -n 1 
commit bb28a88187ca9195884dae319e8a660838de948b (HEAD -> main, origin/main, origin/HEAD)
Author: Antonin Bas <abas@vmware.com>
Date:   Thu Jun 15 18:46:31 2023 -0700

    Only push tagged versions to PyPI (#191)

    Signed-off-by: Antonin Bas <abas@vmware.com>
# git status
On branch main
Your branch is up to date with 'origin/main'.

nothing to commit, working tree clean
# python3 setup.py install --single-version-externally-managed --record /tmp/ptf_install.txt
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/pkg_resources/_vendor/packaging/requirements.py", line 90, in __init__
    req = REQUIREMENT.parseString(requirement_string)
  File "/usr/lib/python3/dist-packages/pkg_resources/_vendor/pyparsing.py", line 1654, in parseString
    raise exc
  File "/usr/lib/python3/dist-packages/pkg_resources/_vendor/pyparsing.py", line 1644, in parseString
    loc, tokens = self._parse( instring, 0 )
  File "/usr/lib/python3/dist-packages/pkg_resources/_vendor/pyparsing.py", line 1402, in _parseNoCache
    loc,tokens = self.parseImpl( instring, preloc, doActions )
  File "/usr/lib/python3/dist-packages/pkg_resources/_vendor/pyparsing.py", line 3417, in parseImpl
    loc, exprtokens = e._parse( instring, loc, doActions )
  File "/usr/lib/python3/dist-packages/pkg_resources/_vendor/pyparsing.py", line 1406, in _parseNoCache
    loc,tokens = self.parseImpl( instring, preloc, doActions )
  File "/usr/lib/python3/dist-packages/pkg_resources/_vendor/pyparsing.py", line 3205, in parseImpl
    raise ParseException(instring, loc, self.errmsg, self)
pkg_resources._vendor.pyparsing.ParseException: Expected stringEnd (at char 4), (line:1, col:5)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/pkg_resources/__init__.py", line 3042, in __init__
    super(Requirement, self).__init__(requirement_string)
  File "/usr/lib/python3/dist-packages/pkg_resources/_vendor/packaging/requirements.py", line 94, in __init__
    requirement_string[e.loc:e.loc + 8]))
pkg_resources.extern.packaging.requirements.InvalidRequirement: Invalid requirement, parse error at "': requir'"

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "setup.py", line 4, in <module>
    setuptools.setup()
  File "/usr/lib/python3/dist-packages/setuptools/__init__.py", line 144, in setup
    _install_setup_requires(attrs)
  File "/usr/lib/python3/dist-packages/setuptools/__init__.py", line 137, in _install_setup_requires
    dist.parse_config_files(ignore_option_errors=True)
  File "/usr/lib/python3/dist-packages/setuptools/dist.py", line 706, in parse_config_files
    self._finalize_requires()
  File "/usr/lib/python3/dist-packages/setuptools/dist.py", line 506, in _finalize_requires
    self._move_install_requirements_markers()
  File "/usr/lib/python3/dist-packages/setuptools/dist.py", line 545, in _move_install_requirements_markers
    inst_reqs = list(pkg_resources.parse_requirements(spec_inst_reqs))
  File "/usr/lib/python3/dist-packages/pkg_resources/__init__.py", line 3035, in parse_requirements
    yield Requirement(line)
  File "/usr/lib/python3/dist-packages/pkg_resources/__init__.py", line 3044, in __init__
    raise RequirementParseError(str(e))
pkg_resources.RequirementParseError: Invalid requirement, parse error at "': requir'"
# git diff
diff --git a/setup.cfg b/setup.cfg
index e1fda97..f8ff662 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -22,7 +22,6 @@ platforms = any
 python_requires = >=3
 setup_requires =
     setuptools_scm
-install_requires = file: requirements.txt

 [options.packages.find]
 where = src
# python3.7 setup.py install --single-version-externally-managed --record /tmp/ptf_install.txt
running install
running build
liushilongbuaa commented 1 year ago
-install_requires = file: requirements.txt

It seems something wrong with this kind of format.

antoninbas commented 1 year ago

@liushilongbuaa I think you are missing my point. The file is correct but requires a more recent version of the setuptools package than what you have.

Regardless, I already said I was ok with the change. As I mentioned above, I am just waiting for you to update the README to remove any mention of requirements.txt, given that you are deleting this file.

antoninbas commented 1 year ago

@liushilongbuaa In addition to updating the README.md file, please also sign the ONF CLA

liushilongbuaa commented 1 year ago

/onf/cla