Bug 1979275

koji scratch build -

https://koji.fedoraproject.org/koji/taskinfo?taskID=71344275

Since pylsp requires pycodestyle, PR updating pyflakes, flake8 and pycodestyle have been submitted. 

Builds can be seen here - https://copr.fedorainfracloud.org/coprs/nonamedotc/spyder5dev/builds/

I rebuilt pyflakes, python-pycodestyle, and python-flake8 in koji.
The think they must be built in this particular order, and that they depend on each other,
because the builds would fail until the previous package was present in the buildroot.
This dependency is not declared in the packages, but it probably should be added.
I.e.:
  python-pycodestyle.spec:Requires: python3-pyflakes >= ...
  python-flake8.spec:Requires: python3-pycodestyle >= ...

But I don't know enough about those packages…

--

The build fails (with the updated packages):

AssertionError: assert b'Content-Length: 10\r\nContent-Type: application/vscode-jsonrpc; charset=utf8\r\n\r\n1546300861' in [b'', b'Content-Length: 10\r\nContent-Type: application/vscode-jsonrpc; charset=utf8\r\n\r\n1546304461', b'Content-Length: 10\r\nContent-Type: application/vscode-jsonrpc; charset=utf8\r\n\r\n1546322461']

No idea if that's significant or not, but please fix ;]

Hi there,


Yes, your order is correct. pyflakes -> pycodestyle -> flake8.



I do not see any build failures for python-lsp-jsonrpc. Here is a build after pycodestyle, etc. were updated - https://koji.fedoraproject.org/koji/taskinfo?taskID=71417694

(In reply to Zbigniew Jędrzejewski-Szmek from comment #3)
> I rebuilt pyflakes, python-pycodestyle, and python-flake8 in koji.
> The think they must be built in this particular order, and that they depend
> on each other,
> because the builds would fail until the previous package was present in the
> buildroot.
> This dependency is not declared in the packages, but it probably should be
> added.
> I.e.:
>   python-pycodestyle.spec:Requires: python3-pyflakes >= ...
>   python-flake8.spec:Requires: python3-pycodestyle >= ...
> 
> But I don't know enough about those packages…

pycodestyle does not require pyflakes

And flake8 already has a complex requires on the others:

(python3.10dist(mccabe) < 0.7 with python3.10dist(mccabe) >= 0.6)
(python3.10dist(pycodestyle) < 2.8 with python3.10dist(pycodestyle) >= 2.7)
(python3.10dist(pyflakes) < 2.4 with python3.10dist(pyflakes) >= 2.3)


Note that mu was broken by the upgrade, I'd appreciate a heads up next time: bz1979411

Spec sanity:


> URL:            https://github.com/python-lsp/%{name}

I find this rather hard to hrasp, especially since the entire spec file rather uses %{short_name}. COnsidder expanding the URL fully, so it is easier to copy paste for whover needs to read it from the spec.


> Source0:        https://files.pythonhosted.org/packages/source/p/%{name}/%{name}-%{version}.tar.gz

Please, use %{pypi_source} instead. The exact URL is not considered future-stable.


> %description
> %_description
> ...
> %description -n python3-%{short_name}
> %_description

The descriptions now start with a newline. Use this instead to avoid it:

  %description %_description
  %description -n python3-%{short_name} %_description



> # Remove bundled egg-info
> rm -rf %{pypi_name}.egg-info

This is not required with pyproject macros. In fact, it is most likely not ever required.



> %check
> pytest

Is it possible to use the documented %pytest macro instead?

> And flake8 already has a complex requires on the others:
>
> (python3.10dist(mccabe) < 0.7 with python3.10dist(mccabe) >= 0.6)
> (python3.10dist(pycodestyle) < 2.8 with python3.10dist(pycodestyle) >= 2.7)
> (python3.10dist(pyflakes) < 2.4 with python3.10dist(pyflakes) >= 2.3)

I does, but apparently those requirements are not enough: the builds failed in tests
when done in the "wrong" order.

> I do not see any build failures for python-lsp-jsonrpc. Here is a build after pycodestyle, etc. were updated - https://koji.fedoraproject.org/koji/taskinfo?taskID=71417694

Hmm, the builds still fail in mock for me. Doesn't matter, I used this mock build for review instead.

+ package name is OK
+ license is acceptable for Fedora (MIT)
+ license is specified correctly
+ builds and installs OK
+ BR/R/P look OK (*)
+ rpmlint shows nothing

In general it's the modern packaging template, so there isn't much to review…
All the things that Miro pointed out above get a +1 from me.
Please consider implementing them when importing.

Package is APPROVED.

(*) I see "python3dist(python-lsp-jsonrpc)" in the autogenerated Provides.
I would expect "python3dist(lsp-jsonrpc)". Not sure what to make of this.

(In reply to Zbigniew Jędrzejewski-Szmek from comment #7)
> (*) I see "python3dist(python-lsp-jsonrpc)" in the autogenerated Provides.
> I would expect "python3dist(lsp-jsonrpc)". Not sure what to make of this.

That's because "python-lsp-jsonrpc" is the actual Python package name. See for example https://pypi.org/project/python-lsp-jsonrpc/

The package does not build for me:

+ pytest
============================= test session starts ==============================
platform linux -- Python 3.10.0b3, pytest-6.2.4, py-1.10.0, pluggy-0.13.1
rootdir: /builddir/build/BUILD/python-lsp-jsonrpc-1.0.0, configfile: setup.cfg, testpaths: test
plugins: cov-2.11.1
collected 27 items

test/test_endpoint.py ......................                             [ 81%]
test/test_streams.py ....F                                               [100%]

=================================== FAILURES ===================================
___________________________ test_writer_bad_message ____________________________

wfile = <_io.BytesIO object at 0x7febc95eba10>
writer = <pylsp_jsonrpc.streams.JsonRpcStreamWriter object at 0x7febc8a5bb50>

    def test_writer_bad_message(wfile, writer):
        # A datetime isn't serializable(or poorly serializable),
        # ensure the write method doesn't throw, but the result could be empty
        # or the correct datetime
        datetime.datetime = JsonDatetime
        writer.write(datetime.datetime(
            year=2019,
            month=1,
            day=1,
            hour=1,
            minute=1,
            second=1,
        ))
    
>       assert wfile.getvalue() in [
            b'',
            b'Content-Length: 10\r\n'
            b'Content-Type: application/vscode-jsonrpc; charset=utf8\r\n'
            b'\r\n'
            b'1546304461',
            b'Content-Length: 10\r\n'
            b'Content-Type: application/vscode-jsonrpc; charset=utf8\r\n'
            b'\r\n'
            b'1546322461'
        ]
E       AssertionError: assert b'Content-Length: 10\r\nContent-Type: application/vscode-jsonrpc; charset=utf8\r\n\r\n1546300861' in [b'', b'Content-Length: 10\r\nContent-Type: application/vscode-jsonrpc; charset=utf8\r\n\r\n1546304461', b'Content-Length: 10\r\nContent-Type: application/vscode-jsonrpc; charset=utf8\r\n\r\n1546322461']
E        +  where b'Content-Length: 10\r\nContent-Type: application/vscode-jsonrpc; charset=utf8\r\n\r\n1546300861' = <built-in method getvalue of _io.BytesIO object at 0x7febc95eba10>()
E        +    where <built-in method getvalue of _io.BytesIO object at 0x7febc95eba10> = <_io.BytesIO object at 0x7febc95eba10>.getvalue

test/test_streams.py:118: AssertionError
- generated xml file: /builddir/build/BUILD/python-lsp-jsonrpc-1.0.0/pytest.xml -

----------- coverage: platform linux, python 3.10.0-beta-3 -----------
Name                           Stmts   Miss  Cover
--------------------------------------------------
pylsp_jsonrpc/__init__.py          2      0   100%
pylsp_jsonrpc/_version.py          2      0   100%
pylsp_jsonrpc/dispatchers.py      19     19     0%
pylsp_jsonrpc/endpoint.py        138      5    96%
pylsp_jsonrpc/exceptions.py       60      6    90%
pylsp_jsonrpc/streams.py          67     14    79%
test/__init__.py                   0      0   100%
test/test_endpoint.py            133      4    97%
test/test_streams.py              49      1    98%
--------------------------------------------------
TOTAL                            470     49    90%
Coverage HTML written to dir htmlcov

=========================== short test summary info ============================
FAILED test/test_streams.py::test_writer_bad_message - AssertionError: assert...
========================= 1 failed, 26 passed in 0.36s =========================

(In reply to Zbigniew Jędrzejewski-Szmek from comment #7)
> > And flake8 already has a complex requires on the others:
> >
> > (python3.10dist(mccabe) < 0.7 with python3.10dist(mccabe) >= 0.6)
> > (python3.10dist(pycodestyle) < 2.8 with python3.10dist(pycodestyle) >= 2.7)
> > (python3.10dist(pyflakes) < 2.4 with python3.10dist(pyflakes) >= 2.3)
> 
> I does, but apparently those requirements are not enough: the builds failed
> in tests
> when done in the "wrong" order.

I see. That means the BuildRequires are too relaxed. I thought you meant the runtime Requires.

(In reply to Miro Hrončok from comment #5)
> Note that mu was broken by the upgrade, I'd appreciate a heads up next time:
> bz1979411

Sorry about that. I did check though ... 

# dnf repoquery --releasever rawhide --whatrequires "python3dist(pycodestyle)"
Last metadata expiration check: 0:02:44 ago on Wed 07 Jul 2021 07:36:17 AM EDT.


For flake8, I got this - 

dnf repoquery --releasever rawhide --whatrequires "python3dist(flake8)"
Last metadata expiration check: 0:02:54 ago on Wed 07 Jul 2021 07:36:17 AM EDT.
python3-molecule-0:3.2.4-2.fc35.noarch
python3-pep8-naming-0:0.11.1-4.fc35.noarch


I checked molecule and pep8-naming. They did not have issues with upgraded versions.

(In reply to Miro Hrončok from comment #9)
> The package does not build for me:
> 
> + pytest
> ============================= test session starts
> ==============================
> platform linux -- Python 3.10.0b3, pytest-6.2.4, py-1.10.0, pluggy-0.13.1
> rootdir: /builddir/build/BUILD/python-lsp-jsonrpc-1.0.0, configfile:
> setup.cfg, testpaths: test
> plugins: cov-2.11.1
> collected 27 items
> 
> test/test_endpoint.py ......................                             [
> 81%]
> test/test_streams.py ....F                                              
> [100%]
> 
> =================================== FAILURES
> ===================================
> ___________________________ test_writer_bad_message
> ____________________________
> 
> wfile = <_io.BytesIO object at 0x7febc95eba10>
> writer = <pylsp_jsonrpc.streams.JsonRpcStreamWriter object at 0x7febc8a5bb50>
> 
>     def test_writer_bad_message(wfile, writer):
>         # A datetime isn't serializable(or poorly serializable),
>         # ensure the write method doesn't throw, but the result could be
> empty
>         # or the correct datetime
>         datetime.datetime = JsonDatetime
>         writer.write(datetime.datetime(
>             year=2019,
>             month=1,
>             day=1,
>             hour=1,
>             minute=1,
>             second=1,
>         ))
>     
> >       assert wfile.getvalue() in [
>             b'',
>             b'Content-Length: 10\r\n'
>             b'Content-Type: application/vscode-jsonrpc; charset=utf8\r\n'
>             b'\r\n'
>             b'1546304461',
>             b'Content-Length: 10\r\n'
>             b'Content-Type: application/vscode-jsonrpc; charset=utf8\r\n'
>             b'\r\n'
>             b'1546322461'
>         ]
> E       AssertionError: assert b'Content-Length: 10\r\nContent-Type:
> application/vscode-jsonrpc; charset=utf8\r\n\r\n1546300861' in [b'',
> b'Content-Length: 10\r\nContent-Type: application/vscode-jsonrpc;
> charset=utf8\r\n\r\n1546304461', b'Content-Length: 10\r\nContent-Type:
> application/vscode-jsonrpc; charset=utf8\r\n\r\n1546322461']
> E        +  where b'Content-Length: 10\r\nContent-Type:
> application/vscode-jsonrpc; charset=utf8\r\n\r\n1546300861' = <built-in
> method getvalue of _io.BytesIO object at 0x7febc95eba10>()
> E        +    where <built-in method getvalue of _io.BytesIO object at
> 0x7febc95eba10> = <_io.BytesIO object at 0x7febc95eba10>.getvalue
> 
> test/test_streams.py:118: AssertionError
> - generated xml file:
> /builddir/build/BUILD/python-lsp-jsonrpc-1.0.0/pytest.xml -
> 
> ----------- coverage: platform linux, python 3.10.0-beta-3 -----------
> Name                           Stmts   Miss  Cover
> --------------------------------------------------
> pylsp_jsonrpc/__init__.py          2      0   100%
> pylsp_jsonrpc/_version.py          2      0   100%
> pylsp_jsonrpc/dispatchers.py      19     19     0%
> pylsp_jsonrpc/endpoint.py        138      5    96%
> pylsp_jsonrpc/exceptions.py       60      6    90%
> pylsp_jsonrpc/streams.py          67     14    79%
> test/__init__.py                   0      0   100%
> test/test_endpoint.py            133      4    97%
> test/test_streams.py              49      1    98%
> --------------------------------------------------
> TOTAL                            470     49    90%
> Coverage HTML written to dir htmlcov
> 
> =========================== short test summary info
> ============================
> FAILED test/test_streams.py::test_writer_bad_message - AssertionError:
> assert...
> ========================= 1 failed, 26 passed in 0.36s
> =========================




It builds for me. Here is one with the correct macro - https://koji.fedoraproject.org/koji/taskinfo?taskID=71450407


+ CFLAGS='-O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -m64 -mcpu=power8 -mtune=power8 -fasynchronous-unwind-tables -fstack-clash-protection'
+ LDFLAGS='-Wl,-z,relro -Wl,--as-needed  -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld '
+ PATH=/builddir/build/BUILDROOT/python-lsp-jsonrpc-1.0.0-1.fc35.noarch/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/sbin
+ PYTHONPATH=/builddir/build/BUILDROOT/python-lsp-jsonrpc-1.0.0-1.fc35.noarch/usr/lib64/python3.10/site-packages:/builddir/build/BUILDROOT/python-lsp-jsonrpc-1.0.0-1.fc35.noarch/usr/lib/python3.10/site-packages
+ PYTHONDONTWRITEBYTECODE=1
+ PYTEST_ADDOPTS=' --ignore=/builddir/build/BUILD/python-lsp-jsonrpc-1.0.0/.pyproject-builddir'
+ /usr/bin/pytest
============================= test session starts ==============================
platform linux -- Python 3.10.0b3, pytest-6.2.4, py-1.10.0, pluggy-0.13.1
rootdir: /builddir/build/BUILD/python-lsp-jsonrpc-1.0.0, configfile: setup.cfg, testpaths: test
plugins: cov-2.11.1
collected 27 items
test/test_endpoint.py ......................                             [ 81%]
test/test_streams.py .....                                               [100%]
- generated xml file: /builddir/build/BUILD/python-lsp-jsonrpc-1.0.0/pytest.xml -
----------- coverage: platform linux, python 3.10.0-beta-3 -----------
Name                           Stmts   Miss  Cover
--------------------------------------------------
pylsp_jsonrpc/__init__.py          2      0   100%
pylsp_jsonrpc/_version.py          2      0   100%
pylsp_jsonrpc/dispatchers.py      19     19     0%
pylsp_jsonrpc/endpoint.py        138      5    96%
pylsp_jsonrpc/exceptions.py       60      6    90%
pylsp_jsonrpc/streams.py          67     14    79%
test/__init__.py                   0      0   100%
test/test_endpoint.py            133      4    97%
test/test_streams.py              49      1    98%
--------------------------------------------------
TOTAL                            470     49    90%
Coverage HTML written to dir htmlcov
============================== 27 passed in 0.58s ==============================

(In reply to Miro Hrončok from comment #6)
> Spec sanity:
> 
> 
> > URL:            https://github.com/python-lsp/%{name}
> 
> I find this rather hard to hrasp, especially since the entire spec file
> rather uses %{short_name}. COnsidder expanding the URL fully, so it is
> easier to copy paste for whover needs to read it from the spec.
> 
> 
> > Source0:        https://files.pythonhosted.org/packages/source/p/%{name}/%{name}-%{version}.tar.gz
> 
> Please, use %{pypi_source} instead. The exact URL is not considered
> future-stable.
> 
> 
> > %description
> > %_description
> > ...
> > %description -n python3-%{short_name}
> > %_description
> 
> The descriptions now start with a newline. Use this instead to avoid it:
> 
>   %description %_description
>   %description -n python3-%{short_name} %_description
> 
> 
> 
> > # Remove bundled egg-info
> > rm -rf %{pypi_name}.egg-info
> 
> This is not required with pyproject macros. In fact, it is most likely not
> ever required.
> 
> 
> 
> > %check
> > pytest
> 
> Is it possible to use the documented %pytest macro instead?



I will implement all these changes during import and update other package reviews as well.

(In reply to Zbigniew Jędrzejewski-Szmek from comment #7)
> > And flake8 already has a complex requires on the others:
> >
> > (python3.10dist(mccabe) < 0.7 with python3.10dist(mccabe) >= 0.6)
> > (python3.10dist(pycodestyle) < 2.8 with python3.10dist(pycodestyle) >= 2.7)
> > (python3.10dist(pyflakes) < 2.4 with python3.10dist(pyflakes) >= 2.3)
> 
> I does, but apparently those requirements are not enough: the builds failed
> in tests
> when done in the "wrong" order.
> 
> > I do not see any build failures for python-lsp-jsonrpc. Here is a build after pycodestyle, etc. were updated - https://koji.fedoraproject.org/koji/taskinfo?taskID=71417694
> 
> Hmm, the builds still fail in mock for me. Doesn't matter, I used this mock
> build for review instead.
> 
> + package name is OK
> + license is acceptable for Fedora (MIT)
> + license is specified correctly
> + builds and installs OK
> + BR/R/P look OK (*)
> + rpmlint shows nothing
> 
> In general it's the modern packaging template, so there isn't much to review…
> All the things that Miro pointed out above get a +1 from me.
> Please consider implementing them when importing.
> 
> Package is APPROVED.
> 
> (*) I see "python3dist(python-lsp-jsonrpc)" in the autogenerated Provides.
> I would expect "python3dist(lsp-jsonrpc)". Not sure what to make of this.



I will implement all of Miro's comment when importing. Thanks for the review. I have submitted a scm-request.


I will also look at the other two packages for similar issues.

(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/python-lsp-jsonrpc

Built on rawhide.