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

feat: introduce cocotb support to conda-forge #21056

Closed
wants to merge 1 commit into from

Conversation

tianrui-wei
Copy link

The recipe is generated with grayskull

Signed-off-by: Tianrui Wei [email protected]

Checklist

  • Title of this PR is meaningful: e.g. "Adding my_nifty_package", not "updated meta.yaml".
  • License file is packaged (see here for an example).
  • Source is from official source.
  • Package does not vendor other packages. (If a package uses the source of another package, they should be separate packages or the licenses of all packages need to be packaged).
  • If static libraries are linked in, the license of the static library is packaged.
  • Package does not ship static libraries. If static libraries are needed, follow CFEP-18.
  • Build number is 0.
  • A tarball (url) rather than a repo (e.g. git_url) is used in your recipe (see here for more details).
  • GitHub users listed in the maintainer section have posted a comment confirming they are willing to be listed there.
  • When in trouble, please check our knowledge base documentation before pinging a team.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/cocotb) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipes/cocotb:

  • License is not an SPDX identifier (or a custom LicenseRef) nor an SPDX license expression.

Documentation on acceptable licenses can be found here.

@tianrui-wei
Copy link
Author

@conda-forge-admin, please ping conda-forge/help-python

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-webservice.

I was asked to ping @conda-forge/help-python and so here I am doing that.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/cocotb) and found some lint.

Here's what I've got...

For recipes/cocotb:

  • Selectors are suggested to take a <two spaces>#<one space>[<expression>] form. See lines [19]

For recipes/cocotb:

  • License is not an SPDX identifier (or a custom LicenseRef) nor an SPDX license expression.

Documentation on acceptable licenses can be found here.

The recipe is generated with grayskull

Signed-off-by: Tianrui Wei <[email protected]>
@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/cocotb) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipes/cocotb:

  • License is not an SPDX identifier (or a custom LicenseRef) nor an SPDX license expression.

Documentation on acceptable licenses can be found here.

@imphil
Copy link

imphil commented Nov 9, 2022

Hi. Philipp from the cocotb project here.

Thanks for helping to make cocotb better. Can you explain what the benefits are of adding cocotb to conda-forge? I'm asking because the majority of support questions we get on the cocotb side are installation-related, and we are working hard to make our installer (as it appears on PyPi) robust and well-tested. Adding another installation variant to the mix is likely to increase our support burden significantly -- hence a closer look at the benefits is warranted.

(Background: cocotb is a bit a "special" Python package, as it is loaded into a Python (libpython) embedded into a (typically closed-source) VHDL/Verilog simulator. Getting all the linking and library discovery right is a major challenge, and the end result is still rather fragile.)

@tianrui-wei
Copy link
Author

Hi Philipp,

Thanks for the reply. The reason I want to add cocotb to conda forge is to enable cocotb in a more reproducible build system like conda. Getting certain library and python versions with pip is still painful even with pip.

Regarding the maintenance burden, I'm happy to fix issues that's conda related should they arrive :)

Best regards,
Tianrui

@imphil
Copy link

imphil commented Nov 9, 2022

The reason I want to add cocotb to conda forge is to enable cocotb in a more reproducible build system like conda. Getting certain library and python versions with pip is still painful even with pip.

That sounds interesting -- can you elaborate on that a bit more? Which kind of reproducibility is conda adding? Is it reproducing binaries from sources?

cocotb ships (since the 1.7 release) binary wheels and tests them in CI against a fair amount of simulators, so we know pretty well which builds users are running.

@tianrui-wei
Copy link
Author

Different from PyPI, conda is more of a general package manager where you can install gcc, standalone python, software libraries etc. You can even ship your own version of iverilog, yosys or verilator via a conda package. It also supports pinning package versions with conda-lock.

@tianrui-wei
Copy link
Author

@conda-forge-admin, please ping conda-forge/staged-recipe

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-webservice.

I was asked to ping @conda-forge/staged-recipe and so here I am doing that.

@carterbox
Copy link
Member

Yes, conda is a general package manager. PYPI was not intended to distribute non-python packages, whereas conda supports package management for libraries/binaries that are non-python.

Comment on lines +44 to +47
license: BSD-4-Clause & BSD-3-Clause
license_file:
- LICENSE
- cocotb/_vendor/tcl/license.terms
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is incomplete. You need to include the license information for ALL of the revendored source code.

https://github.com/cocotb/cocotb/tree/master/cocotb/_vendor

Comment on lines +30 to +31
run:
- python >=3.6
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs say that this package requires a simulator. Where are the simulators?

Copy link
Member

@timkpaine timkpaine Mar 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iverilog/verilator are 2 supported simulators, both already available via conda forge

sha256: 1f390bb6468d70e04acebeb9ff05b54be42cbbbbbb3bd6c75d1ec0a36cb97e23

build:
noarch: python
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This package is not noarch. It contains compiled code.

Suggested change
noarch: python

test:
imports:
- cocotb
commands:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines +16 to +18
missing_dso_whitelist:
- '*/libgcc_s.so.1'
- '*/libstdc++.so.6'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This package contains compiled C code. Why would you whitelist missing C libraries?

skip:
- true # [win or osx]

requirements:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timkpaine
Copy link
Member

timkpaine commented Mar 16, 2023

I'm probably going to open a similar PR shortly unless we want to finish fixing up this one, I can list @tianrui-wei as maintainer as well

@timkpaine timkpaine mentioned this pull request Mar 18, 2023
10 tasks
@timkpaine
Copy link
Member

@carterbox / @tianrui-wei ive resolved all (?) except the license comments on my PR, will fix the licenses shortly (need to find how the IEEE LRM code is licensed)

@tianrui-wei
Copy link
Author

Hi @timkpaine, sorry for the delay in response and thanks for fixing the outstanding issues. One reason I kind of gave up on this PR is that it contains source code licensed by Siemens Mentor, and it's not clear to me whether it's compatible with open source licenses that conda forge accepts. Thanks for the effort anyway :)

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

Successfully merging this pull request may close these issues.

6 participants