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

Add Apicula for Gowin FPGA Devices #11

Merged
merged 4 commits into from
Apr 8, 2021
Merged

Conversation

racerxdl
Copy link
Contributor

@racerxdl racerxdl commented Jan 15, 2021

This adds hdlc/apicula and hdlc/pkg:apicula for using in Gowin FPGA Devices.

Based on discussion in #4

This shouldn't affect any other package.

Copy link
Member

@umarcor umarcor left a comment

Choose a reason for hiding this comment

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

Looks good with a couple of minor fixes (see below). However, updating the docs is pending. See https://hdl.github.io/containers/#_step_by_step_checklist. It's ok if you don't want to update the graph. Apicula is not listed in the To Do. So, you just need to:

  • Add apicula to hdl/awesome. It's a markdown file with a yaml frontmatter. Minimal fields are project name, a description and an URL to a repo/site.
  • Update dopc/tools.yml, including the ref to the awesome page, and the images the tool is included in.
  • Add shield/badge to section CI.

Let us know if you need help with any of them 😃

@umarcor
Copy link
Member

umarcor commented Jan 16, 2021

I just had a look at https://github.com/hdl/containers/pull/11/checks?check_run_id=1711703969#step:6:53 (the log of dockerTestPkg apicula), and it seems empty. You might need to use the following:

curl -fsSL https://codeload.github.com/YosysHQ/apicula/tar.gz/master | tar xzf - --strip-components=1
python3 setup.py build
python3 setup.py install --prefix=/usr/local --root=/tmp/apicula --optimize=1 --skip-build

@racerxdl
Copy link
Contributor Author

Well its not that easy to compile the apicula by hand, it needs the gowin sdk and extract all the stuff. The PIP package is already pre-compiled. https://github.com/YosysHQ/apicula/blob/master/Dockerfile

The test is empty for now, it doesn't do anything. The image works correctly since it's what I'm using locally. I will make the fixes :D

@umarcor
Copy link
Member

umarcor commented Jan 17, 2021

Well its not that easy to compile the apicula by hand, it needs the gowin sdk and extract all the stuff. The PIP package is already pre-compiled. https://github.com/YosysHQ/apicula/blob/master/Dockerfile

It is not necessary to compile it by hand, specially if that requires gowin's sdk (which might not be free). The point is to do whatever it needs to be done, so that all the artifacts/assets are available in $DESTDIR/user/local/, where DESTDIR=/opt/apicula or DESTDIR=/tmp/apicula. The motivation is to then copy the content of that directory to multiple containers, without installing pip in each of them.

Therefore, if you want to download the pre-compiled PIP package, and rearrange the artifacts, that's an acceptable solution. Building from sources would be ideal, but not an stopper.

/cc @pepijndevos, we are trying to install apicula to a directory, which is then to be copied into different environments with Python but not pip. For reference, Symbiyosys supports make DESTDIR=/opt/symbiyosys install. Can you provide us any guideline about how to achieve it with apicula?

The test is empty for now, it doesn't do anything. The image works correctly since it's what I'm using locally. I will make the fixes :D

I meant the dockerTestPkg, not the dockerTest. That is hdlc/pkg:apicula is empty, but hdlc/apicula works because you installed it explicitly with pip. In other words, in the current state, hdlc/pkg:apicula is useless (and unused). We need to be useful, because other PRs (#10) will need it.

@pepijndevos
Copy link

Uh, if you don't want pip you can just get the source tarball and use the setup.py I suppose?

Or you can just copy the source tarball somewhere and just run it from there. You just need to make sure to run with python -m apycula.gowin_pack to set the path as expected. You could generate your own wrapper scripts if you really want to avoid installing it as a python package. The only thing to watch out for is that if you do a naive python apycula/gowin_pack.py it wont be able to find all the stuff, see YosysHQ/apicula#25

@racerxdl
Copy link
Contributor Author

I think I found a nice way, python let's change the "root" of its work by setting a virtualenv. Adding that makes all thing it install go that folder instead of another one. That should work :D

@racerxdl
Copy link
Contributor Author

racerxdl commented Jan 17, 2021

Hey @pepijndevos! My worry is that the picke files are not at the repository just in the pip package, so the gowin_pack / gowin_unpack wouldn't work I think.

@umarcor umarcor added enhancement New feature or request $R/impl Related to `$R/impl` images and their dependencies $R/pkg Related to `$R/pkg` images and their dependencies labels Jan 18, 2021
@umarcor umarcor mentioned this pull request Jan 18, 2021
@pepijndevos
Copy link

I mean the pypi source tarball https://pypi.org/project/Apycula/#files

@racerxdl
Copy link
Contributor Author

Ahh @pepijndevos , I will try the pypi package directly. This should work :D

@racerxdl
Copy link
Contributor Author

racerxdl commented Feb 6, 2021

Hey @umarcor I think I got it now

&& DEBIAN_FRONTEND=noninteractive apt-get -y install --no-install-recommends python3 python3-setuptools

COPY --from=build /opt/apicula /opt/apicula
RUN cd /opt/apicula && python3 setup.py install
Copy link
Member

Choose a reason for hiding this comment

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

This RUN command should not be done here, but in the build stage. Therefore, python3-setuptools should not be required in the final image.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well there is no way to install apycula without that. It is needed for that image otherwise the gowin_pack and gowin_unpack will not be usable. These commands are aliases to python functions that is created by setup tools.

Copy link
Member

Choose a reason for hiding this comment

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

Well, that is undesirable, because it requires a non standard procedure for this tool, which we need to replicate in all the images where apicula is to be used. I would strongly recommend having the build procedure fixed/enhanced upstream, so that it's installable similarly to Symbiyosys.

Anyway, let's move forward. Please, do the following modifications:

Choose a reason for hiding this comment

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

You can actually run the scripts with python -m apycula.gowin_pack without installation, the trick is just that the module path has to be set correctly else the databases can't be found. The installation step just adds a platform-specific binary file, but in this case you could include your own wrapper script if desired.

@umarcor
Copy link
Member

umarcor commented Feb 7, 2021

WRT to CI errors, rebasing and force pushing should get rid of them. Those were fixed in main yesterday.

@racerxdl racerxdl force-pushed the apicula-rb branch 3 times, most recently from 1980ce9 to f6e1468 Compare February 27, 2021 17:35
@racerxdl
Copy link
Contributor Author

I wish github actions could use other images built in the same pipeline, so apicula would work. Maybe I should add python3-setuptools in another PR?

umarcor added a commit that referenced this pull request Apr 8, 2021
@umarcor umarcor force-pushed the apicula-rb branch 4 times, most recently from b473de3 to 055086c Compare April 8, 2021 03:47
@umarcor
Copy link
Member

umarcor commented Apr 8, 2021

I rebased and updated. There seems to be some issue with CI evaluating conditions incorrectly when I push to a PR. Anyway, I'll merge it. The solution is not ideal for now, but it will allow users to pull and use */apicula, while the wrappers for making it independent from pip, setuptools and wheel are contributed.

Thanks!

@umarcor umarcor merged commit dc35b64 into hdl:main Apr 8, 2021
umarcor added a commit that referenced this pull request Apr 8, 2021
@racerxdl racerxdl deleted the apicula-rb branch April 8, 2021 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request $R/impl Related to `$R/impl` images and their dependencies $R/pkg Related to `$R/pkg` images and their dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants