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

Pin Java version + add support for Java 9, 10, 11 #5

Merged
merged 30 commits into from
Dec 1, 2018

Conversation

hadim
Copy link
Member

@hadim hadim commented Nov 22, 2018

@hanslovsky: Here is a first try to pin the Java version.

@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 (recipe) and found some lint.

Here's what I've got...

For recipe:

  • The source section contained an unexpected subsection name. string is not a valid subsection name.

@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 (recipe) and found it was in an excellent condition.

@hadim
Copy link
Member Author

hadim commented Nov 22, 2018

@jakirkham & @sodre: does this approach make sense to you?

@sodre
Copy link
Member

sodre commented Nov 22, 2018

@hadim, if you are trying to build pyjnius against different versions of java, e.g. 8, 9, 10, 11, etc.. here is how I did it within the context of postgis and postgres.

  1. I kept the postgis version and build strings intact as you would normally do with any package.

  2. I added postgres as a host and run time dependencies in the meta.yaml file without any version information.

  3. I created the conda_build_config.yaml file pinning the postgres versions.

  4. Finally, I reran the conda smithy rerender command to get the additional ci_support files for CircleCI and Travis.

@sodre
Copy link
Member

sodre commented Nov 22, 2018

I just saw your conda_build_config.yaml and meta.yaml files. I think you just need to add the max_pin to conda_build_config.yaml, remove the build.string field, and rerun codna smithy rerender to get this to work.

@hadim
Copy link
Member Author

hadim commented Nov 22, 2018

@hanslovsky : ready for review/merge.

@hadim
Copy link
Member Author

hadim commented Nov 22, 2018

Good news: the next version of pyjnius will be compatible with Java >=9: kivy/pyjnius#363

@hadim
Copy link
Member Author

hadim commented Nov 24, 2018

This is a work in progress: DO NOT MERGE.

@hadim hadim changed the title Pin Java version Pin Java version + add support for Java 9, 10, 11 Nov 24, 2018
@hadim
Copy link
Member Author

hadim commented Nov 26, 2018

But since it does not depend on a compiler, why it is not available on some of them?

@sodre
Copy link
Member

sodre commented Nov 26, 2018

I think it is because it has not gone through the GCC7 Migration yet, e.g. the build number is <1000

@hadim
Copy link
Member Author

hadim commented Nov 26, 2018

Ok, so it's an easy fix I guess.

Can I migrate myself? Or do I have to wait for the bot?

@hadim
Copy link
Member Author

hadim commented Nov 26, 2018

I checked https://conda-forge.org/status/ and couldn't find openjdk in it. I guess it's because no compiler is declared in its recipe.

As already said the compiler is only declared in the test section. Is that why a compiler is declared in the yaml configuration?

A solution would be to remove tests that use C compiler, that would make the package compiler-independent. On the other side, I think it's a good practice to check that JNI compilation works.

@hadim
Copy link
Member Author

hadim commented Nov 28, 2018

This PR should fix the issue conda-forge/openjdk-feedstock#42

Once it's merged I will remove the hard-coded C compilers from this PR.

@hadim
Copy link
Member Author

hadim commented Nov 28, 2018

To do before merging here:

@hadim
Copy link
Member Author

hadim commented Nov 28, 2018

It's almost good. Only OSX builds for toolchain_c are failing when solving env:

conda.exceptions.ResolvePackageNotFound: 
  - openjdk=10

It's weird because conda-forge has those builds. Example for Java 11: https://anaconda.org/conda-forge/openjdk/files?version=11.0.1. There is OSX builds for both toolchain_c and clang with main and gcc7 labels. I also checked inside those packages and info/hash_input.json contains (for the main labeled package):

{
  "c_compiler": "toolchain_c"
}

Does Anaconda repos have an index that could have been confused by the java branches I have created on the Java feedstock repo? See https://github.com/conda-forge/openjdk-feedstock/branches

@hadim
Copy link
Member Author

hadim commented Nov 29, 2018

I guess conda servers have been fixed because the missing OSX OpenJDK packages are back.

It's all green now.

Now waiting for a pyjnius release.

@hadim
Copy link
Member Author

hadim commented Nov 29, 2018

@sodre could you review, please?

Copy link
Member

@sodre sodre left a comment

Choose a reason for hiding this comment

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

@hadim this looks fine to me minus the build_number jinja2 expression and the fact that we are still pulling from github.

Keep in mind that I am not a maintainer of this package so I can't really merge this into the master branch.

recipe/meta.yaml Outdated Show resolved Hide resolved
recipe/meta.yaml Outdated Show resolved Hide resolved
@hadim
Copy link
Member Author

hadim commented Dec 1, 2018

@sodre Is that ok to release a dev version in master here? I feel it could take a while before we see a git tag on the pyjnius side.

Something like 1.1.3-dev0-GIT_HASH?

@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 (recipe) and found some lint.

Here's what I've got...

For recipe:

  • Package version 1.1.3_dev0-b212d5 doesn't match conda spec

@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 (recipe) and found it was in an excellent condition.

@sodre
Copy link
Member

sodre commented Dec 1, 2018

@sodre Is that ok to release a dev version in master here? I feel it could take a while before we see a git tag on the pyjnius side.

Something like 1.1.3-dev0-GIT_HASH?

That is probably a question better asked to @conda-forge/core ...

@hadim
Copy link
Member Author

hadim commented Dec 1, 2018

I got a green light on gitter from @CJ-Wright to merge here.

@hanslovsky only can merge this PR and I know he is AFK for two weeks.

@CJ-Wright: could you merge this PR (if you can...)?

@CJ-Wright
Copy link
Member

@hadim any interest in joining the maintainers of this package?

@hadim
Copy link
Member Author

hadim commented Dec 1, 2018

I will be once this PR is merged :-)

@CJ-Wright
Copy link
Member

sorry I missed that bit

@CJ-Wright CJ-Wright merged commit 680b7f4 into conda-forge:master Dec 1, 2018
@hadim hadim deleted the pinjava branch December 1, 2018 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants