cocotb package

[themperek]

Simple setup.py that allow packaging cocotb.
It allows installing with pip install cocotb==1.0.dev3.

• change build folder to a local directory (no need to have access to installation directory)
• one can get the path to COCOTB with $(shell cocotb-config --share) (no need to set COCOTB environment variable)
• remove SPHINX_BUILD environmental variable and replace with COCOTB_SIM to recognize if loading simulation. This makes easy multi-platform support.
• using tox to run the tests

I have modified examples but not tests.
After merge will automatically package build.
It should be backward compatible (symlink to makefile folder)

[themperek]

One improvement would be to precompile the libraries during installation. For windows distribute precompiled libraries. This will work if no customization is need during simulation later?

[stuarthodgson]

@themperek This stands a good chance of getting pulled in as is after I have tested in our development environment. As you also state I wholly agree about pre compiling binaries as well during "instalation" but pip is really no good at that. Maintaining pre-compiled libs is also not a path I have much interest in going down given how much that can bolloon (also do not use windows). Hope to pull this in relatively soon or at least make some suggestions.

[themperek]

I can work on this and make it Windows compatible but would need to have the possibility to import cocotb without setting any environmental variable (SPHINX_BUILD) during installation.

This can be important change but can we recognise the simulation based on COCOTB_SIM variable (if set then simulation) and remove SPHINX_BUILD variable?

[themperek]

Should now work with: pip install cocotb

In Makefile use:

include $(shell cocotb-path)/makefiles/Makefile.inc
include $(shell cocotb-path)/makefiles/Makefile.sim

[forensicgarlic]

I'm a little unclear from reading the updates, this works fine for linux, but might have windows issues?

[elms]

@themperek I was able to remove the need for SPHINX_BUILD here. Although I realized that by moving where the makefiles, lib, and include are installed (under site-packages/cocotb where I feel they should be), that they are now inconsistent with the structure of the repository.

@stuarthodgson Would you be open to moving the support files under the cocotb module as they are all in support of that python module?

[stuarthodgson]

I don't have a problem with it. Can someone explain to me what we get by having cocotb pip installable but still needed to have the libs compiled?

[TC01]

If nothing else, on multi-user systems it makes it really easy to ensure everyone is using the same version of cocotb. (Even without installing through pip, moving the build folder to the local directory makes it far easier to have one copy/specific version of cocotb used by multiple people).

[elms]

pip also helps with python dependencies, but I don't that applies to cocotb at the moment.

cocotb is unique in how python is invoked and interacts with the simulators. I think it would definitely be worth while if a move toward a more pythonic approach (e.g. #437).

[themperek]

@elmsfu
To make cocotb-path.exe on windows one would need to make cocotb-path by console entry.
Something like:

setup(
    entry_points={
        'console_scripts': [
            'cocotb-path= cocotb.cocotb:main',
        ]
    }
)

For this one needs to call import cocotb without error. Maybe there is some workaround but I do not know any at this point.

@stuarthodgson Can we recognize the simulation based on COCOTB_SIM variable (if set then simulation) and remove SPHINX_BUILD variable to get better windows support?

[themperek]

I don't have a problem with it. Can someone explain to me what we get by having cocotb pip installable but still needed to have the libs compiled?

• so that you do not have to set any extra environmental variable or download anything
• so it is easy to install/update/unistall
• so cocotb is easy to add as requirements for other packages

Next step for me would be to precompile the libraries during installation so one could provide packages without need of compilers etc.

[imphil]

This looks like a good step forward, thanks for the effort @themperek.

However, I think it's crucial to add the compilation of the libraries to the packaging as well. Today the compilation step is a major pain point (in my experience), and it becomes even more so if users use "sudo pip install cocotb" and then try to run cocotb as normal user, in which case they cannot compile the libraries as they don't have (or should have) access to the installation directory.
Including the compilation step does not mean shipping pre-compiled libraries, this is orthogonal and optional.

But, since perfect is the enemy of good, I'm open to take this PR as-is, and improve the situation in further PRs later on.

@themperek Before merging can you clean up the commit history a bit using git rebase -i?

[TC01]

However, I think it's crucial to add the compilation of the libraries to the packaging as well. Today the compilation step is a major pain point (in my experience), and it becomes even more so if users use "sudo pip install cocotb" and then try to run cocotb as normal user, in which case they cannot compile the libraries as they don't have (or should have) access to the installation directory.

Part of this pull request is to make it so that the libraries are compiled locally (in your current working directory) instead of in the cocotb installation directory-- a local "build" folder gets created when running. So this is something that's already fixed-- unless I'm missing something?

One other thing, though, is that this appears to pollute the python/site-packages directory:

$ ls ~/.local/lib/python2.7/site-packages/ -l
total 132
drwxrwxr-x.  5 bjr bjr  4096 Nov 14 14:52 cocotb
drwxrwxr-x.  2 bjr bjr  4096 Nov 14 14:52 cocotb-1.0.1-py2.7.egg-info
drwxrwxr-x.  2 bjr bjr  4096 Nov 14 14:52 include
drwxrwxr-x. 11 bjr bjr  4096 Nov 14 14:52 lib
drwxrwxr-x.  3 bjr bjr  4096 Nov 14 14:52 makefiles
drwxrwxr-x.  2 bjr bjr  4096 Sep 17 15:04 tests

If it's installed in a virtualenv this isn't such a problem, but it would be really nice to install the include, lib, and makefiles folders below "cocotb" rather than into site-packages directly.

[themperek]

@imphil

However, I think it's crucial to add the compilation of the libraries to the packaging as well.

It is compiled locally now. If you mean to compile during setup then this is not so easy. As I understand libraries are specific to the simulator and interface. One could precompile for all available simulators but not sure if this is much better. Distributing binaries, in this case, would be hard.

[themperek]

@TC01

One other thing, though, is that this appears to pollute the python/site-packages directory.

The only easy way I see is to move makfiles, lib and include into cocotb folder (maybe cocotb/src). Can do if others agree.

[stuarthodgson]

I'd just like to try before merging. Also I think we already have 1.0 as a tag but that won't match this 1.0 soaybe we merge, tag, then do what is needed on pypi at 1.1.

[themperek]

One can install via: pip install cocotb==1.0.dev2

In Makefile use:

include $(shell cocotb-path)/makefiles/Makefile.inc
include $(shell cocotb-path)/makefiles/Makefile.sim

PyPi: https://pypi.org/project/cocotb/1.0.dev2/

[imphil]

@imphil

However, I think it's crucial to add the compilation of the libraries to the packaging as well.

It is compiled locally now. If you mean to compile during setup then this is not so easy. As I understand libraries are specific to the simulator and interface. One could precompile for all available simulators but not sure if this is much better. Distributing binaries, in this case, would be hard.

Ah ok, I misread the code when skimming over it. Sounds good!

[imphil]

Regarding the versioning thing: even though it's not fantastic, setuptools_scm works reasonable well (https://github.com/pypa/setuptools_scm) and makes sure the SCM version is equal to the package version, even in local and development builds.

[themperek]

I'd just like to try before merging

@stuarthodgson Any news?

[stuarthodgson]

Some but only on a related note of syncing our internal fork with upstream in preparation for a big or so we can drop out internal fork and move to upstream again. Which will mean it gets more focus. I'll try tomorrow.to test this

[imphil]

While thinking about the directory structure I realized that we can actually follow a bit more established conventions by renaming "cocotb-path" to "cocotb-config", and giving it command arguments like "--makefiles" to get the path to the makefiles, or cocotb-config --libdir to get the libdir, or "--incdir" to get the include directory. This is in line of how tools used to do it before "pkg-config" provided an universal solution for that. For example, ncurses6-config outputs that on my machine:

$ ncurses6-config --help
Usage: ncurses6-config [options]

Options:
  --prefix           echos the package-prefix of ncurses
  --exec-prefix      echos the executable-prefix of ncurses

  --cflags           echos the C compiler flags needed to compile with ncurses
  --libs             echos the libraries needed to link with ncurses

  --version          echos the release+patchdate version of ncurses
  --abi-version      echos the ABI version of ncurses
  --mouse-version    echos the mouse-interface version of ncurses

  --bindir           echos the directory containing ncurses programs
  --datadir          echos the directory containing ncurses data
  --includedir       echos the directory containing ncurses header files
  --libdir           echos the directory containing ncurses libraries
  --mandir           echos the directory containing ncurses manpages
  --terminfo         echos the $TERMINFO terminfo database path
  --terminfo-dirs    echos the $TERMINFO_DIRS directory list
  --termpath         echos the $TERMPATH termcap list

  --help             prints this message

[themperek]

• I have changed cocotb-path to cocotb-config at the moment it just returns path to share folder. Shout it be rather cocotb-config --prefix or cocotb-config --makefiles ?
• I have kept symbolic links to share folders. I think it is better to keep backward compatibility for some time. Could add a warning.
• If it is OK I would add tox in a separate PR.
• Not sure why travis fails at the moment on 2.7.

[imphil]

Thanks @themperek, nice! I didn't look to closely yet (I'll do that tomorrow), just as a couple quick starters:

I have changed cocotb-path to cocotb-config at the moment it just returns path to share folder. Shout it be rather cocotb-config --prefix or cocotb-config --makefiles ?

It would be probably good to add the arg parsing feature from the beginning, otherwise we run into backwards-compat problems again as soon as we add them.

I have kept symbolic links to share folders. I think it is better to keep backward compatibility for some time. Could add a warning.

If users specify COCOTB=/path/to/cocotb/srcdir/cocotb/share and run their existing Makefiles, doesn't that do the trick already? So the only thing users need to do is set a different COCOTB environment variable if they update their cocotb installation. That sounds like a simple-enough change to me not to warrant a symlink. (Since you cannot be surprised by that, you actively need to update cocotb in the first place.)

If it is OK I would add tox in a separate PR.

Sure. However it might actually help solving the current problems with CI (see below).

Not sure why travis fails at the moment on 2.7.

The tests fail because the "cocotb" module cannot be found, so most likely the PYTHONPATH path is wrong -- which shouldn't need to be set anywhere in the first place if the package is installed properly.
Also Dockerfile currently sets COCOTB=xxx, this can probably go as well.

Let me know if you get stuck, I'll have a closer look tomorrow.

[themperek]

I have kept symbolic links to share folders. I think it is better to keep backward compatibility for some time. Could add a warning.

If users specify COCOTB=/path/to/cocotb/srcdir/cocotb/share and run their existing Makefiles, doesn't that do the trick already? So the only thing users need to do is set a different COCOTB environment variable if they update their cocotb installation. That sounds like a simple-enough change to me not to warrant a symlink. (Since you cannot be surprised by that, you actively need to update cocotb in the first place.)

If one has to change this then also can do COCOTB=$(cocotb-path --share) but this would need python setup.py develop/install or pip install cocotb before.

I would suggest releasing before we merge this (1.0.1?) and then have a new version (1.1?) with package support (upload to pipy) and update documentation. Let me know what you think.

[imphil]

Looks great, thanks Tomasz for continuing the long journey here! I've added a couple minor comments, but overall looks very good to me. Let's see what others say about the version/release question and then I hope we can ship this next week! Hooray!

[imphil]

I've tested this and the package looks good to me. Let's keep this as-is for now and not add more changes to it. The commit history is already a bit hard to follow and it would be nice to get it rebased and cleaned up into logical commits (e.g. one for each add cocotb-config, move files, use cocotb-config, and finally the travis changes). Let me know if you need help, it requires a bit of git experience.

[themperek]

The commit history is already a bit hard to follow and it would be nice to get it rebased and cleaned up into logical commits

@imphil Not sure if I can do this "into logical commits" apart form squash all together or make a new pull request.

[eric-wieser]

Not sure how a new PR would help here @themperek, unless your goal is to start with a clean slate regarding comments.

One approach I often take to cleaning up difficult history full of iteration is:

• git tag in-case-I-screw-up
• git reset --mixed 4aab2d90b4e5b7fd057b185a7b234dc87397301a (the commit you started at)
• Repeat until no changes remain:
  • git add -p, to add things piece by piece (easier through a GUI tool IMO)
  • git commit

[themperek]

@eric-wieser This seems to be not so straight since there was a rebase in the middle.

[imphil]

No worries, I'll do the rebase in the coming days.

[themperek]

In case someone wants to try: pip install cocotb==1.0.dev3

[imphil]

With the 1.1 release out now I'll look at this one again. I'll rebase this PR in the coming days and get it merged, ETA end of the week.

[Matthewar]

With PEP517 being supported by pip now, should we be using this instead of setuptools?

[imphil]

I've looked through this PR and split it up and rebased it on top of the current master. I'll now step-by-step open PRs for the individual changes in the hope that this will lead to the least breakage for this rather large PR. This included also a couple small functional changes, I'll highlight them as we move along.

The current patch series is in https://github.com/imphil/cocotb/commits/packaging-rebased. Note that this series will be rebased as we iterate through the sub-PRs.

[imphil]

Starting now with #799 and #800 as first small independent fixes. More to follow as soon as these two have gone in, awaiting review by @themperek.

[cmarqu]

I guess we should add at least the following classifiers here:

    classifiers=[
        "Programming Language :: Python :: 2.7",
        "Programming Language :: Python :: 3",
        "License :: OSI Approved :: BSD License",
        "Topic :: Scientific/Engineering :: Electronic Design Automation (EDA)",
    ],

[themperek]

Good point. Can you make a PR for this? Now it is in master.