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 klayout #20396

Closed
wants to merge 18 commits into from
Closed

add klayout #20396

wants to merge 18 commits into from

Conversation

curtisma
Copy link
Contributor

@curtisma curtisma commented Sep 12, 2022

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 wanted to let you know that I linted all conda-recipes in your PR (recipes/klayout) and found some lint.

Here's what I've got...

For recipes/klayout:

  • Failed to even lint the recipe, probably because of a conda-smithy bug 😢. This likely indicates a problem in your meta.yaml, though. To get a traceback to help figure out what's going on, install conda-smithy and run conda smithy recipe-lint . from the recipe directory.

@curtisma
Copy link
Contributor Author

I need you both to comment on this PR stating that you agree to be a maintainer.

@klayoutmatthias
@proppy

"${SRC_DIR}/build.sh"
echo "bin-release Contents"
echo "--------------------"
ls bin-release
Copy link
Contributor

Choose a reason for hiding this comment

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

find? (if you want to make sure to list everything recursively?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll update that if necessary for debug.

recipes/klayout/meta.yaml Outdated Show resolved Hide resolved
recipes/klayout/meta.yaml Outdated Show resolved Hide resolved
recipes/klayout/meta.yaml Outdated Show resolved Hide resolved
# GitHub IDs for maintainers of the recipe.
# Always check with the people listed below if they are OK becoming maintainers of the recipe. (There will be spam!)
- curtisma
- klayoutmatthias
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree to be co-maintainer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

@curtisma curtisma mentioned this pull request Sep 12, 2022
8 tasks
recipes/klayout/meta.yaml Outdated Show resolved Hide resolved
recipes/klayout/meta.yaml Outdated Show resolved Hide resolved
@curtisma
Copy link
Contributor Author

I'm trying to submit #20396 which requires a python ".a" file, for example it's looking in the following location:
$PREFIX/lib/x86_64-linux-gnu/libpython3.10.a

Normally I think this would come from a OS package such as python-dev or python-devel. Is there a way to get this ".a" python library file with a Conda dependency I can include in my meta.yaml?

@curtisma
Copy link
Contributor Author

curtisma commented Sep 26, 2022

We'll need to try using the following package:
libpython-static

@curtisma
Copy link
Contributor Author

@joamatab, are you willing to be added as a maintainer as well?

@curtisma
Copy link
Contributor Author

Well I was able to fix the error about the missing pythonlib ".a" file.

@curtisma
Copy link
Contributor Author

curtisma commented Sep 26, 2022

Next error shown below. Seems to be an issue with the setup of the C compiler and finding the make file.
I'm first focusing on linux-64

qmake $SRC_DIR/src/klayout.pro -recursive CONFIG+=release RUBYLIBFILE=$PREFIX/lib/libruby.so.3.1.2 RUBYVERSIONCODE=30102 HAVE_RUBY=1 PYTHON=$PREFIX/bin/python PYTHONLIBFILE=$PREFIX/lib/libpython3.10.a PYTHONINCLUDE=$PREFIX/include/python3.10 PYTHONEXTSUFFIX=.cpython-310-x86_64-linux-gnu.so HAVE_PYTHON=1 HAVE_QTBINDINGS=1 HAVE_QT_UITOOLS=1 HAVE_QT_NETWORK=1 HAVE_QT_SQL=1 HAVE_QT_SVG=1 HAVE_QT_PRINTSUPPORT=1 HAVE_QT_MULTIMEDIA=1 HAVE_QT_DESIGNER=1 HAVE_QT_XML=1 HAVE_64BIT_COORD=0 HAVE_QT=1 HAVE_QT5=1 HAVE_CURL=0 HAVE_EXPAT=0 PREFIX=$SRC_DIR/bin-release RPATH= KLAYOUT_VERSION=0.27.11 KLAYOUT_VERSION_DATE=2022-09-26 KLAYOUT_VERSION_REV=8e85a9e256 RUBYINCLUDE=$PREFIX/include/ruby-3.1.0 RUBYINCLUDE2=$PREFIX/include/ruby-3.1.0/x86_64-linux
Project ERROR: Cannot run compiler 'g++'. Output:
===================
===================
Maybe you forgot to setup the environment?

Running build (make  all) ..
make: *** No targets specified and no makefile found.  Stop.
Traceback (most recent call last):
  File "/home/conda/staged-recipes-copy/.ci_support/build_all.py", line 223, in <module>
    build_all(os.path.join(root_dir, "recipes"), args.arch)
  File "/home/conda/staged-recipes-copy/.ci_support/build_all.py", line 120, in build_all
    build_folders(recipes_dir, folders, arch, channel_urls)
  File "/home/conda/staged-recipes-copy/.ci_support/build_all.py", line 179, in build_folders
    conda_build.api.build([recipe], config=get_config(arch, channel_urls))
  File "/opt/conda/lib/python3.9/site-packages/conda_build/api.py", line 186, in build
    return build_tree(
  File "/opt/conda/lib/python3.9/site-packages/conda_build/build.py", line 3088, in build_tree
    packages_from_this = build(metadata, stats,
  File "/opt/conda/lib/python3.9/site-packages/conda_build/build.py", line 2211, in build
    utils.check_call_env(cmd, env=env, rewrite_stdout_env=rewrite_env,
  File "/opt/conda/lib/python3.9/site-packages/conda_build/utils.py", line 411, in check_call_env
    return _func_defaulting_env_to_os_environ('call', *popenargs, **kwargs)
  File "/opt/conda/lib/python3.9/site-packages/conda_build/utils.py", line 391, in _func_defaulting_env_to_os_environ
    raise subprocess.CalledProcessError(proc.returncode, _args)
subprocess.CalledProcessError: Command '['/bin/bash', '-o', 'errexit', '/home/conda/staged-recipes/build_artifacts/klayout_1664206022399/work/conda_build.sh']' returned non-zero exit status 1.
##[error]Bash exited with code '1'.
Finishing: Run docker build

@joamatab
Copy link
Contributor

Hi Curtis, Im happy to also be a maintainer

Thomas made it available on PyPI
@thomaslima

and Floris has it on his conda environment
@flaport

@flaport
Copy link
Contributor

flaport commented Sep 26, 2022

This looks great, if possible I'd like to help to maintain this too.

on another note... do you think we could package the klayout-gui too? Something like this: https://anaconda.org/flaport/klayout-gui

@curtisma
Copy link
Contributor Author

This looks great, if possible I'd like to help to maintain this too.

Ok, sure, I'll add you also.

on another note... do you think we could package the klayout-gui too? Something like this: https://anaconda.org/flaport/klayout-gui

Nice! I'll have to take a look at your work there I was intending this to include the gui version of klayout. Then we could possibly create a second recipe called "klayout-python" with just the python package as a lighter weight option for Python users.

Should the "klayout" package include just the gui version or also include the python package?

At the moment I'm still working through getting it to compile. It'd be great if you are able to help since you have experience with compiling it previously. I've submitted several Conda recipes in the past but this is the first time I've compiled klayout. Using conda-forge to handle the CI and CD should help make it easier to host the package and update it. It will also make it easier for users to find it compared to having it on your own channel.

@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/klayout) and found some lint.

Here's what I've got...

For recipes/klayout:

  • Failed to even lint the recipe, probably because of a conda-smithy bug 😢. This likely indicates a problem in your meta.yaml, though. To get a traceback to help figure out what's going on, install conda-smithy and run conda smithy recipe-lint . from the recipe directory.

@klayoutmatthias
Copy link

I tried to debug the issue with qmake.

For short: I do not fully understand what's going on, but I have a patch that should make linux64 work. Here is my patched build.sh for "klayout" package:

#!/bin/bash

# even though we specify QMAKE_CXX in -expert mode, qmake still
# needs "g++" while obtaining gcc paths (while bootstrapping?)
# So we create a temporary link called "g++"
mkdir tmp_exe
cd tmp_exe
ln -s $GXX g++
cd ..
export PATH=$(pwd)/tmp_exe:$PATH 

${SRC_DIR}/build.sh -python ${PYTHON} -bin ${PREFIX}/bin -expert

echo "bin Contents"
echo "--------------------"
ls "${PREFIX}/bin"

First I added -expert (BUILD_EXPERT=1) which takes all tools from the conda environment. Still I get the Cannot run compiler 'g++' error. Looking at the qmake source code, it should be taken from QMAKE_CXX (hence from the conda environment). Yet it is using the mkspecs default (g++). I assume this is a kind of bootstrap step and the configuration is not evaluated yet at this point. So the hack is to create a g++ symbolic link pointing to the actual g++ tool.

Maybe there is a more elegant solution, but the above hack seems to work.

@klayoutmatthias
Copy link

klayoutmatthias commented Nov 13, 2022

Update: the above patch makes qmake work, but the build fails because the OpenGL libraries are not found .. :(

Honestly, I don't know how to get libGL into the conda environment.

The following article says that additional Linux packages need to be installed outside conda: https://docs.anaconda.com/anaconda/install/linux/

@curtisma
Copy link
Contributor Author

curtisma commented Nov 13, 2022

Thanks for the help @klayoutmatthias!

Update: the above patch makes qmake work, but the build fails because the OpenGL libraries are not found .. :(

I found the following section in the conda-forge docs about handling libGL dependencies:
https://conda-forge.org/docs/maintainer/knowledge_base.html#libgl

Maybe I'll just start by adding all of those items to the requirements? Unless we only need a subset of them, what do you think?
I'll also add the yum_requirements.txt in case it is needed for testing as the docs suggested. I noticed they have that in the pyopengl recipe. Though they only have mesa-libGL rather than that full list.

I hope to apply the patch suggested and these libgl updates later this evening.

@thomaslima
Copy link

@curtisma curtisma changed the title add klayout add klayout-gui 1 hour ago

I'd be in favor to keep klayout as the default name as I think most people would expect the gui to be present when installing the package from conda-forge (just like the klayout packages from other general purpose distribution).

Maybe we could have a different package (klayout-python?) for the Python API only version, wdyt?

Hard disagree on the default name. I think klayout as default for conda shouldn't come with GUI features. I think the gui should be an optional install. Surely there is a way to resolve this without going to two different names. Doesn't conda support optional build options? like conda install klayout --with-gui?

If two different names are required, I strongly suggest the one with more features have a longer name. Like klayout-gui superseding klayout (python only). Could it be possible to have klayout-gui have klayout as dependency?

Thanks,
Thomas

@proppy
Copy link
Contributor

proppy commented Nov 13, 2022

I think klayout as default for conda shouldn't come with GUI features.

@thomaslima see hdl/conda-eda#193 (comment) for a bit more background about my suggestion to have the conda-forge package be full featured: we also plan to have a recipe as part of conda-eda that provide a klayout package with minimal dependencies (and just depending on the main channel).

Doesn't conda support optional build options? like conda install klayout --with-gui?

conda-build has a feature called "outputs" that allow defining subpackages: https://docs.conda.io/projects/conda-build/en/latest/resources/define-metadata.html#outputs-section

Maybe we could leverage it here:

  • klayout.cli: klayout cli without the gui
  • klayout.gui: klayout cli with gui
  • klayout.python: klayout pythoon bindings (no cli, and qtless feature as part of 0.28)

Then we could make the klayout one be a meta packages for klayout.python + one of the other two (if we people most people would be interested by having a functioning klayout binary + some python bindings).

What do you think?

@curtisma
Copy link
Contributor Author

@proppy, The outputs feature is interesting. Hoever I don't think it's well-known though. At least I haven't seen it in use in any other recipes. So I'd prefer to keep it simple and seperate them into different packages. Hopefully there are no conflicts between the Python and gui packages so both can be installed. We should probably test that out though.

Another option is we we call them "klayout-python" and "klayout-gui" to keep them descriptive and not have a "klayout" package.

@curtisma
Copy link
Contributor Author

curtisma commented Nov 14, 2022

A larger goal of mine is to provide a more integrated open-source design environment. We can do this by having the Viper IC Design Environment (IDE) provide the "library / project manager" gui along with an easy project and environment setup CLI. Then other Conda packages, including Klayout and xschem, will provide individual tools as extensions to open and edit cell views. It should remain lightweight but extensible and customizable, similar to VS Code.

Another goal would be to have a full design environment setup and configured in less than 15mins based on a simple set of instructions with less than 10 steps.
This could be done using an IC-design-specific Conda distribution, with a single installer, using Conda Constructor, similar to the data science Anaconda distribution and miniforge.

There could be different flavors of the distribution, one for analog design, digital design, and integrated photonics. Each would be a different set of packages. Each could have both an installer and a simple environment.yaml to create a new environment in an existing conda installation. Some could also support using proprietary tools as necessary. This was the idea behind viper-forge.

What do you guys think? Do you agree with these goals? Are there others that should be added?
@proppy, @mithro, @joamatab @thomaslima @klayoutmatthias @flaport

@proppy
Copy link
Contributor

proppy commented Nov 14, 2022

Another goal would be to have a full design environment setup and configured in less than 15mins based on a simple set of instructions with less than 10 steps.
This could be done using an IC-design-specific Conda distribution, with a single installer, using Conda Constructor, similar to the data science Anaconda distribution and miniforge.

This is really aligned with what we discussed here:
hdl/conda-eda#245 (comment)
And there is a proof of concept here for digital design:
hdl/conda-eda#247

Let's continue the discussion there? (and keep this issue focused on klayout?)

@proppy
Copy link
Contributor

proppy commented Nov 14, 2022

The outputs feature is interesting. Hoever I don't think it's well-known though

From the point of view of the user there is no different with a standalone packages: the . in the package name is just a convention, other package like https://github.com/conda-forge/opencv-feedstock/blob/main/recipe/meta.yaml#L205 use totally different names; so this is more of a convenience to be able to maintain multiple packages from the same feedstock which I believe is what we want to do here.

@curtisma
Copy link
Contributor Author

curtisma commented Nov 14, 2022

hmm, seems like we may need to rerun the build. It's throwing the following error:
conda.CondaMultiError: Downloaded bytes did not match Content-Length

From the log

@proppy
Copy link
Contributor

proppy commented Nov 14, 2022

@conda-forge-admin please rerender

@klayoutmatthias
Copy link

Thanks for this wonderful discussion! :)

Just my 2 cents:

klayout.gui and klayout.python make sense to me as package names. klayout.cli makes sense if you like to provide a standalone runtime for DRC for example. However, klayout.gui contains a buddy tool called strmrun which essentially does the same.

If klayout.cli is about reducing dependencies I need to say I hardly used KLayout without Qt. I think it works (I have one CI build for that), but I'm not entirely sure.

Thanks a lot for all your efforts!

Matthias

@thomasdorch
Copy link

thomasdorch commented Nov 21, 2022

Hi all, I have made some progress on getting a conda build for Windows working . I had to go around the built.bat script since it requires klayout-bits, which isn't necessary to build the GUI on Windows. It's not extensively tested but it works on my machine. Let me know if/where I should submit a PR if it's helpful!

@klayoutmatthias
Copy link

I have also made some progress with the conda package for Linux (https://github.com/klayoutmatthias/staged-recipes/tree/klayout). It builds on the main channel, but not yet with conda-forge. I had to change a few things compared to this PR.

It produces a conda package file which tests successfully, but trying to install it with "--use-local" makes conda stall in an endless loop to fulfill requirements. No idea if that is a bug in the package or in conda.

I can file a PR if you like.

Matthias

@thomasdorch
Copy link

@klayoutmatthias I think that may have to do with the way dependencies are versioned in the meta.yaml. According to the conda package match spec, there needs to be a space in-between the package name and any relational operators:

Remember that the version specification cannot contain spaces, as spaces are used to delimit the package, version, and build string in the whole match specification. python >= 2.7 is an invalid match specification. Furthermore, python>=2.7 is matched as any version of a package named python>=2.7.

i.e. instead of Qt>=5, it needs to be Qt >=5

@thomaslima
Copy link

Thanks for the discussion. I have to say I am not too familiar with the conda packaging process. I thought one recipe meant one install process. Sounds like this sub packaging would be the way to go.

Moreover, I was also confused about the three versions cli, gui and python. I don't know what cli does. Can anyone give an example? I still vote for .python being the default, since at least to my understanding conda is python package manager. The full featured could be .gui.

One quick clarification: would installing klayout.gui be a functional replacement for installing from a binary downloaded from klayout.de?

@proppy
Copy link
Contributor

proppy commented Dec 21, 2022

I still vote for .python being the default, since at least to my understanding conda is python package manager.

conda has strong tie in the python ecosystem initially but is now more of a general cross platform packaging system and distribute a lot of packages with standalone binaries and native libraries.

@proppy
Copy link
Contributor

proppy commented Dec 21, 2022

One quick clarification: would installing klayout.gui be a functional replacement for installing from a binary downloaded from klayout.de?

That would be an alternative for the conda-forge audience (similar to how https://www.klayout.de/build.html lists packages for different Linux distributions).

@basnijholt basnijholt mentioned this pull request Mar 29, 2023
10 tasks
@stale
Copy link

stale bot commented May 22, 2023

Hi friend!

We really, really, really appreciate that you have taken the time to make a PR on conda-forge/staged-recipes! conda-forge only exists because people like you donate their time to build and maintain conda recipes for use by the community.

In an effort to maintain this repository and increase the signal-to-noise for open PRs, the maintainers of staged-recipes close excessively old PRs after six months. This PR will remain open for another month, and then will be closed.

If you'd like to keep it open, please comment/push and we will be happy to oblige! Note that very old PRs will likely need to be rebased on main so that they can be rebuilt with the most recent CI scripts. If you have any trouble, or we missed reviewing this PR in the first place (sorry!), feel free to ping the team using a special command in a comment on the PR to get the attention of the staged-recipes team.

Cheers and thank you for contributing to this community effort!

@stale stale bot added the stale will be closed in 30 days label May 22, 2023
Copy link

stale bot commented Jan 1, 2024

Hi again! About a month ago, we commented on this PR saying it would be closed in another month if it was still inactive. It has been a month and so now it is being closed. Thank you so much for making it in the first place and contributing to the community project that is conda-forge. If you'd like to reopen this PR, please feel free to do so at any time!

Cheers and have a great day!

@stale stale bot closed this Jan 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale will be closed in 30 days
Development

Successfully merging this pull request may close these issues.

8 participants