Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prebuilt Binary Release to PyPi #187

Open
khwong-c opened this issue Jan 5, 2024 · 12 comments
Open

Prebuilt Binary Release to PyPi #187

khwong-c opened this issue Jan 5, 2024 · 12 comments
Milestone

Comments

@khwong-c
Copy link
Contributor

khwong-c commented Jan 5, 2024

Hi Nic

I found hdlConvertor interesting for my coming project (on Python generated SV), and would like to use it.
However, I would like my user can use hdlConvertor directly from a simple pip install hdlConvertor, without building it from the source every time. (e.g. apt install ... and the build process during pip install hdlConvertor)

So, I would like to have a built version of hdlConvertor with ANTLR4 library and I am more than happy to contribute.

We might expect the following in the Wheel bundle:

  • Compiled hdlConvertor shared library, with Python Wrapper
  • Compiled ANTLR4 shared library
  • License for both libraries included

Example: Test PyPi page

I can provide the GitHub Action to achieve this by:

  1. Integrating the GitHub CI into this repository, OR
  2. Hosting another repository and publishing as a separate package, e.g. hdlConvertor-binary

May I have your thoughts on this proposal?
Looking forward to contributing with you 😄

@Nic30
Copy link
Owner

Nic30 commented Jan 8, 2024

@khwong-c

If you can provide config for any CI which builds binary packages it would be helpful.
It is obvious that the installation takes long and is memory intense. It complicates integration as well as testing of dependent projects.

Currently i replaced cmake with meson to get rid of issues which you are describing. https://github.com/Nic30/hdlConvertor/tree/mesonbuild
My only problem now is to get coverage reporting working again and then I merge mesonbuild to master.
So if you can, lets work with that branch only.

Currently I am saturated. So if you can help me with fixing this issue I will be super happy otherwise I will have to do it alone, probably in the time of yet another worst crisis of my life.

It would be ideal if we can:

  • build binary for several common pythons/distros
  • build binary for windows
  • have src package with automatically builds/downloads antlr4 using meson
    • code coverage reporting is fixed for meson build
  • have everything working in CI
    • upload src and binary packages to PyPi

@khwong-c
Copy link
Contributor Author

khwong-c commented Jan 8, 2024

@Nic30
I am not familiar with Meson build. So I couldn't help with the Meson build for you.

However, I can build Binary Python bundles for most of the Python + Linux Distro with your original setup.py script (with CMake).

So what I can offer is:

  • Build binary for several common pythons/distros
    • Windows players still need to compile from the source.
  • Have Python Wheel package with ANTLR4 included.
  • Upload src and binary packages to PyPi
  • Have items above working in GitHub CI.

I will submit an MR

@khwong-c
Copy link
Contributor Author

khwong-c commented Jan 8, 2024

@Nic30
Sorry, I missed one point from the last comment.

May I ask for the intention of moving from CMake to Meson?
What are the reasons for changing the build system?

@eli-schwartz
Copy link

My only problem now is to get coverage reporting working again and then I merge mesonbuild to master.

What's the issue you're having with coverage? https://mesonbuild.com/Unit-tests.html#coverage

@Nic30
Copy link
Owner

Nic30 commented Jan 10, 2024

@eli-schwartz

What's the issue you're having with coverage?

It seems to me like this mesonbuild/meson#9585 but it should be fixed.

https://github.com/Nic30/hdlConvertor/blob/mesonbuild/.circleci/config.yml#L51
https://app.circleci.com/pipelines/github/Nic30/hdlConvertor/157/workflows/f7e5cb0a-257e-4925-af32-df570d79eef6/jobs/223

I do not know how to force meson to enable coverage for _hdlConvertor*.so in order to generate .gcda coverage report files.

I am would like to do something like this https://shenxianpeng.github.io/2021/07/gcov-example/

Currently we are not using meson defined tests, in this project the meson is used only to build python c++ extension (_hdlConvertor*.so) which is then used in python tests, then the coverage report data should be collected and uploaded to coveralls.

But the problem is that the .so generated from meson does not generate .gcda when used from python (which means that it was not build with correct option to enable coverage).
Basically what we need is to force meson to enable coverage for mentioned target. It would be best if we can do this somehow universally so it is not compiler dependent (optional).

@Nic30
Copy link
Owner

Nic30 commented Jan 10, 2024

@khwong-c

May I ask for the intention of moving from CMake to Meson?
What are the reasons for changing the build system?

Building and using ANTRL4 from Meson is more simple, readable and reliable. (It was more easy to move to meson than implement all optional downloads and path patching than in cmake to make ANTRL4 work reliably on any machine.)
Description in meson is much shorter, cleaner and the meson system for subproject is better than equivalent in cmake.
Meson matured enough to build python extensions reliably.

@khwong-c
Copy link
Contributor Author

@Nic30 PR submitted.

@Nic30 Nic30 added this to the v2.4 milestone Jan 14, 2024
@Nic30
Copy link
Owner

Nic30 commented Jan 15, 2024

@eli-schwartz I asked in meson matrix about this https://app.element.io/#/room/#mesonbuild:matrix.org I probably have everything solved. Currently the only problem is how to add .gcno files to whl or how to build in-tree. Because currently coverage files are not preserved because of how build builds staff. I did not find any option for in-tree build in that library.

@Nic30
Copy link
Owner

Nic30 commented May 21, 2024

Status update:

mesonbuild branch is now working for several months, but I did not find time to fix circleci orb publishing circleci-orb-python-all-in-1.
(The orb contains universal scripts for project building, testing and publishing. )
The problem with the orb is that the automatic version generating stops working if long time has passed from last version.
There are also issues with github credentials/hooks every 5 months or so. In January I spend 2 days fixing it, but it did not seem to be any of common issues. Then I found out a new forum thread claiming that the issue is on CircleCI side, then I postponed it and 4 months have passed away in the blink of the eye.

This obscurity currently preventing me from merging of meson branch which will provide binary packages and compatibility with newer antlr.
I do not know if this is sign of Microsoft fighting against Circleci, Circleci or circleci-orb-python-all-in-1 being broken
however it really breaks on regular basis and I am considering rewriting it to dockeker or repo with bash scripts.

If someone is reading this, and you have some best practice for maintaining CI for large number of python/js/ts/c++ libraries, I would be happy to hear it.

@Thomasb81
Copy link
Contributor

I can't comment on automation running on circleci for publishing, but it seems to me that current head of mensonbuild branch fail to pass the testsuite.

  ======================================================================
FAIL: testSimpleSubunit (tests.test_basic_hdl_sim_model_from_verilog.BasicHdlSimModelFromVerilogTC.testSimpleSubunit)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/root/project/tests/test_basic_hdl_sim_model_from_verilog.py", line 22, in testSimpleSubunit
    self.parseWithRef("simple_subunit.v", VERILOG,
  File "/root/project/tests/hdl_parse_tc.py", line 134, in parseWithRef
    self.assertEqual(ref, res_str)
AssertionError: 'from[922 chars]elf._hwIOs = (\n            self.io.a,\n      [2433 chars]\n\n' != 'from[922 chars]elf._interfaces = (\n            self.io.a,\n [2415 chars]\n\n'
Diff is 3982 characters long. Set self.maxDiff to None to see it.

======================================================================
FAIL: test_decoder_using_case (tests.test_verilog_to_hwt.VerilogToHwtTC.test_decoder_using_case)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/root/project/tests/test_verilog_to_hwt.py", line 34, in test_decoder_using_case
    self.parseWithRef("decoder_using_case.v")
  File "/root/project/tests/test_verilog_to_hwt.py", line 29, in parseWithRef
    return HdlParseTC.parseWithRef(self, fname, language,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/root/project/tests/hdl_parse_tc.py", line 134, in parseWithRef
    self.assertEqual(ref, res_str)
AssertionError: 'from[84 chars]hwt.hwIOs.std import HwIOSignal\nfrom hwt.hwMo[2474 chars] )\n' != 'from[84 chars]hwt.hdl.types.array import HArray\nfrom hwt.hd[2466 chars] )\n'
Diff is 3304 characters long. Set self.maxDiff to None to see it.

======================================================================
FAIL: test_uart (tests.test_verilog_to_hwt.VerilogToHwtTC.test_uart)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/root/project/tests/test_verilog_to_hwt.py", line 43, in test_uart
    self.parseWithRef("uart.v")
  File "/root/project/tests/test_verilog_to_hwt.py", line 29, in parseWithRef
    return HdlParseTC.parseWithRef(self, fname, language,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/root/project/tests/hdl_parse_tc.py", line 134, in parseWithRef
    self.assertEqual(ref, res_str)
AssertionError: 'from[84 chars]hwt.hwIOs.std import HwIOSignal\nfrom hwt.hwMo[5946 chars] )\n' != 'from[84 chars]hwt.hdl.types.array import HArray\nfrom hwt.hd[5893 chars] )\n'
Diff is 7917 characters long. Set self.maxDiff to None to see it.

----------------------------------------------------------------------
Ran 9564 tests in 177.908s

FAILED (failures=3)

github workflow is still running and not more align with build procedure.

@Nic30
Copy link
Owner

Nic30 commented Aug 26, 2024

Mentioned test are failing because other library was just updated (and reference file is just old).

There is like 10h of work (circle-ci tuning, package checking) to get mesonbuild branch to work and to publish a new package, yet there is always some urgent stuff to do, currently until 15th. This is really annoying.

@Thomasb81
Copy link
Contributor

Mentioned test are failing because other library was just updated (and reference file is just old).

How can we progress on this ? hdlConvertorAst 1.2 is the last published version if I am correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants