Add PyJNIus recipe #6091

hanslovsky · 2018-06-14T18:11:07Z

Closes #5075

conda-forge recipe for PyJNIus. Currently WIP, trying to figure out Windows build.

conda-forge-linter · 2018-06-14T18:11:19Z

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

Here's what I've got...

For recipes/pyjnius:

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.

mariusvniekerk · 2018-06-14T18:24:28Z

recipes/pyjnius/meta.yaml

+  version: {{tagged_version}}
+
+source:
+  git_url: https://github.com/kivy/pyjnius.git


Can you point to a git archive for this instead?

For now we need this, we have asked upstream for a new release. ( kivy/pyjnius#342 ) The last one is too old (from a year ago).

Since there are some build issues that need to get sorted here, we are hoping to work on this asynchronously while the release is being sorted out. Hopefully we can incorporate it at the end.

Should add that once we do have a release, we will be able to drop the Jinja above, which should fix the linter issue as well.

We could take a git archive from an arbitrary sha. Though i guess that doesn't differ much

It's true. Not to worried about it as this should be transient anyways.

mariusvniekerk · 2018-06-14T18:24:49Z

recipes/pyjnius/meta.yaml

+
+build:
+  number: 0
+  string: py{{py}}_{{PKG_BUILDNUM}}


please remove the build string

mariusvniekerk · 2018-06-14T18:25:45Z

recipes/pyjnius/meta.yaml

+   - {{ compiler('cxx') }}
+ host:
+   - openjdk >=8
+   - python {{PY_VER}}*


remove the {{PYVER}}* bit here since we don't need it

mariusvniekerk · 2018-06-14T18:27:49Z

recipes/pyjnius/meta.yaml

+ host:
+   - openjdk >=8
+   - python {{PY_VER}}*
+   - cython


you'll want to add pip here

Think this is causing the build failure on Linux

jakirkham · 2018-06-14T19:15:35Z

recipes/pyjnius/meta.yaml

+   - ant
+ run:
+   - openjdk >=8
+   - python {{PY_VER}}*


This one should be dropped too

jakirkham · 2018-06-14T19:16:21Z

recipes/pyjnius/meta.yaml

+about:
+  home: https://github.com/kivy/pyjnius
+  license: MIT
+  license_file: LICENSE 


Also needs the maintainers section with your GitHub handle. Snippet from the example below.

staged-recipes/recipes/example/meta.yaml

Lines 75 to 80 in a6f81a0

extra:

recipe-maintainers:

# 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!)

- LisaSimpson

- LandoCalrissian

- ignore python2 - add separate activate/deactivate files - fix PATH variable on Windows

hanslovsky · 2018-06-14T21:26:53Z

We were able to fix the issues with the Windows build. Shoutout to @jjahanip and @jakirkham for their help! @jjahanip setting PATH variable was what fixed it in the end!

jakirkham · 2018-06-14T21:49:18Z

Thanks for working on this, Phil. Glad we were able to get Windows building. 😄

jakirkham · 2018-06-15T01:35:41Z

recipes/pyjnius/activate.bat

+SET "JDK_HOME_BACKUP=%JDK_HOME%"
+SET "JDK_HOME=%JAVA_HOME%"
+SET "PATH_BACKUP=%PATH%"
+SET "PATH=%JDK_HOME%\jre\bin\server;%JDK_HOME%\bin;%PATH%"


Since %JDK_HOME% is just the %LIBRARY_PREFIX%, %JDK_HOME%\bin is already on the %PATH% as conda adds that for us. Given this, it appears the critical thing is having %JDK_HOME%\jre\bin\server added. Inspecting that directory reveals only one library, which is jvm.dll (needed by libraries using JNI). This explains the loading error and why we needed to add this to the %PATH%. Have submitted PR ( conda-forge/openjdk-feedstock#30 ) to have openjdk do this for us.

Another thing worth checking, it appears on macOS and Linux set JAVA_LD_LIBRARY_PATH to the equivalent location. Maybe setting this alone would also solve the issue on Windows?

jakirkham · 2018-06-15T01:37:22Z

recipes/pyjnius/activate.bat

+SET "PYJNIUS_JAR_BACKUP=%PYJNIUS_JAR%"
+SET "PYJNIUS_JAR=%CONDA_PREFIX%\share\pyjnius\pyjnius.jar"
+SET "JDK_HOME_BACKUP=%JDK_HOME%"
+SET "JDK_HOME=%JAVA_HOME%"


Would be curious to see if we can just drop the %JDK_HOME% stuff given we didn't need it on Unix and it seems like we have a better understanding of the problem now.

The PyJNIus doc states that we should set it, so I'd just leave it in there.
http://pyjnius.readthedocs.io/en/latest/installation.html#installation-for-windows

jakirkham · 2018-06-15T01:38:07Z

recipes/pyjnius/activate.sh

@@ -0,0 +1,2 @@
+export PYJNIUS_JAR_BACKUP=$PYJNIUS_JAR
+export PYJNIUS_JAR=$CONDA_PREFIX/share/pyjnius/pyjnius.jar


Minor comment: Would be good to quote these paths in the event the user has spaces in them.

conda-forge-linter · 2018-07-17T15:43:57Z

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/pyjnius) and found it was in an excellent condition.

hanslovsky · 2018-07-17T19:06:54Z

I udpated meta.yaml to comply with the linter. All checks pass now. What is the way forward here?

jakirkham · 2018-07-20T15:05:50Z

Seems fine to me. Thoughts @conda-forge/staged-recipes?

scopatz · 2018-07-20T15:24:20Z

LGTM Thanks!

Add PyJNIus recipe

4dacc51

mariusvniekerk reviewed Jun 14, 2018

View reviewed changes

Fix nosetests typo

8b6062a

jakirkham reviewed Jun 14, 2018

View reviewed changes

recipes/pyjnius/meta.yaml Outdated

- ant

run:

- openjdk >=8

- python {{PY_VER}}*

Copy link

Member

jakirkham Jun 14, 2018

Choose a reason for hiding this comment

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

This one should be dropped too

jakirkham reviewed Jun 14, 2018

View reviewed changes

jakirkham mentioned this pull request Jun 14, 2018

Any news for 1.1.2? kivy/pyjnius#342

Closed

hanslovsky added 3 commits June 14, 2018 15:42

Address review comments in #6091

696b0c2

Add setuptools as runtime dependency (pkg_resources)

f76f8e9

Fix issues with builds:

7171e00

- ignore python2 - add separate activate/deactivate files - fix PATH variable on Windows

hanslovsky mentioned this pull request Jun 14, 2018

Cannot import imglyb in init method imagej/pyimagej#6

Closed

jakirkham mentioned this pull request Jun 14, 2018

Package PyJNIus #5075

Closed

jakirkham reviewed Jun 15, 2018

View reviewed changes

hanslovsky added 2 commits June 15, 2018 23:04

Quote paths/variables in case of spaces

76d2890

Comply conda smithy

f85e132

scopatz merged commit 9cd51b6 into conda-forge:master Jul 20, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add PyJNIus recipe #6091

Add PyJNIus recipe #6091

hanslovsky commented Jun 14, 2018 •

edited by jakirkham

conda-forge-linter commented Jun 14, 2018

mariusvniekerk Jun 14, 2018

jakirkham Jun 14, 2018

jakirkham Jun 14, 2018

mariusvniekerk Jun 14, 2018

jakirkham Jun 15, 2018

mariusvniekerk Jun 14, 2018

mariusvniekerk Jun 14, 2018

mariusvniekerk Jun 14, 2018

jakirkham Jun 14, 2018

jakirkham Jun 14, 2018

jakirkham Jun 14, 2018

hanslovsky commented Jun 14, 2018

jakirkham commented Jun 14, 2018

jakirkham Jun 15, 2018

jakirkham Jun 15, 2018

jakirkham Jun 15, 2018

hanslovsky Jul 17, 2018

jakirkham Jun 15, 2018

conda-forge-linter commented Jul 17, 2018

hanslovsky commented Jul 17, 2018

jakirkham commented Jul 20, 2018

scopatz commented Jul 20, 2018

	extra:
	recipe-maintainers:
	# 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!)
	- LisaSimpson
	- LandoCalrissian

		@@ -0,0 +1,2 @@
		export PYJNIUS_JAR_BACKUP=$PYJNIUS_JAR
		export PYJNIUS_JAR=$CONDA_PREFIX/share/pyjnius/pyjnius.jar

Add PyJNIus recipe #6091

Add PyJNIus recipe #6091

Conversation

hanslovsky commented Jun 14, 2018 • edited by jakirkham

conda-forge-linter commented Jun 14, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hanslovsky commented Jun 14, 2018

jakirkham commented Jun 14, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

conda-forge-linter commented Jul 17, 2018

hanslovsky commented Jul 17, 2018

jakirkham commented Jul 20, 2018

scopatz commented Jul 20, 2018

hanslovsky commented Jun 14, 2018 •

edited by jakirkham