added recipe for openjdk #5792

boussaffawalid · 2021-06-07T14:47:55Z

Added recipe for openjdk
inspired by https://github.com/conan-io/conan-center-index/tree/master/recipes/zulu-openjdk

I've read the guidelines for contributing.
I've followed the PEP8 style guides for Python code in the recipes.
I've used the latest Conan client version.
I've tried at least one configuration locally with the
conan-center hook activated.

recipes/openjdk/all/conanfile.py

prince-chrismc · 2021-06-07T14:52:55Z

recipes/openjdk/all/conanfile.py

+                  destination=self._source_subfolder, strip_root=True)
+
+    def build(self):
+        pass # nothing to do, but this shall trigger no warnings ;-)


We have a no_copy_source to skip this

prince-chrismc · 2021-06-07T15:50:07Z

Please do not force push we need to restart the reviewing

prince-chrismc · 2021-06-07T15:51:14Z

You need to open an issue against the hooks to add this as an exception.

You can see the Zulu or for more information

boussaffawalid · 2021-06-07T15:57:05Z

@prince-chrismc conan-io/hooks#306

jgsogo

I need to ask if this recipe is needed by some other recipe. According to our packaging policy, we allow only packages that are built from sources. If they are needed for other PRs or libraries, better to say it in the description of the PR.

Thanks!

boussaffawalid · 2021-06-23T13:37:34Z

@jgsogo this recipe is not needed by any other other recipe. I prepared this recipe as I needed it for my own project and I thought it may be useful for other people as well.

SSE4 · 2021-06-30T09:07:32Z

AFAIK, there is no official binaries of OpenJDK, anyone (e.g. Zulu) can make his own distribution from the sources.
e.g. Microsoft now has started to to that as well: https://www.microsoft.com/openjdk
the list is: https://en.wikipedia.org/wiki/OpenJDK#OpenJDK_builds

ericriff · 2021-07-21T00:42:09Z

recipes/openjdk/all/conanfile.py

+        return "source_subfolder"
+
+    def configure(self):
+        if self.settings.arch != "x86_64":


This checks should go in validate()
https://docs.conan.io/en/latest/reference/conanfile/methods.html#validate

ericriff · 2021-07-21T00:46:37Z

recipes/openjdk/all/test_package/conanfile.py

+from conans.errors import ConanException
+from io import StringIO
+
+required_conan_version = ">=1.36.0"


What happens if you're running an older version? The main recipe works and then it errors out during test?

It raises a ConanException error: https://docs.conan.io/en/latest/reference/conanfile/other.html#requiring-a-conan-version-for-the-recipe

Yes I know it raises an error, but I'm talking about the placement.
I usually see this line in the main conanfile.py. By having it on the test_package's conanfile.py, would you allow the package to build and then error out during test?
Are both conanfiles evaluated before build?

test_package is not evaluated before the build

so should I add the required_conan_version to the main conanfile ?

I think you are correct here.
The main recipe needs conan 1.33. -> you need to add it there too
The test recipe needs conan 1.36.

it does not make sense to require two different versions. lets use 1.36 in main recipe

You can build the main recipe without a test recipe (-tf None) or build with a different test recipe (-tf other_test_package).

This is ok. For a client using 1.33 it will fail in a conan create command, but it is accurate and, as said before, you can avoid running these tests, or use a different test folder... and a conan install ... --build=openjdk won't fail, which is the command you would run when retrieving this recipe from a remote (test_package is not uploaded to the remote).

the assumption is that OP reacted to the review and applied the required changes! Testing locally this caught 7 PRs including conan-io/conan-center-index#5792 which was well over due

prince-chrismc · 2021-08-28T02:58:33Z

Refining my bot caught this PR.... it had lots of approvals and OP applied the required changes...

Please review again!

madebr · 2021-08-28T03:29:43Z

recipes/openjdk/all/test_package/conanfile.py

+    test_type = "build_requires"
+
+    def build(self):
+        pass  # nothing to build, but tests should not warn


Perhaps compiling something JNI?

madebr · 2021-08-28T03:30:07Z

recipes/openjdk/all/test_package/conanfile.py

+
+    def test(self):
+        output = StringIO()
+        self.run("java --version", output=output, run_environment=True)


I think this should be run conditionally, when not cross building.

stale · 2021-09-27T04:29:08Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

jgsogo

We talked about this PR some time ago. We decided that as long as the tool is required to build some other project or used by other projects, it would be ok to redistribute already built packages. Of course, if building from sources is not possible at the moment and redistribution is allowed by the licensing.

recipes/openjdk/all/test_package/conanfile.py

jgsogo · 2021-09-27T07:33:01Z

recipes/openjdk/all/test_package/conanfile.py

+from conans.errors import ConanException
+from io import StringIO
+
+required_conan_version = ">=1.36.0"


This is ok. For a client using 1.33 it will fail in a conan create command, but it is accurate and, as said before, you can avoid running these tests, or use a different test folder... and a conan install ... --build=openjdk won't fail, which is the command you would run when retrieving this recipe from a remote (test_package is not uploaded to the remote).

Co-authored-by: Javier G. Sogo <jgsogo@gmail.com>

recipes/openjdk/all/test_package/conanfile.py

Co-authored-by: Chris Mc <prince.chrismc@gmail.com>

conan-center-bot · 2021-09-27T15:14:09Z

All green in build 17 (9e093ced819bd7f6a765629a3dd46269e1b3cbaf):

openjdk/16.0.1@:
All packages built successfully! (All logs)

jgsogo

LGTM.

We can improve the recipe in following PRs (checks that should go to validate())

This comment has been minimized.

Sign in to view

conan-center-bot added the Failed label Jun 7, 2021

prince-chrismc suggested changes Jun 7, 2021

View reviewed changes

boussaffawalid force-pushed the openjdk branch from 2d342f5 to 7178919 Compare June 7, 2021 15:07

This comment has been minimized.

Sign in to view

uilianries mentioned this pull request Jun 7, 2021

Add KB-H013 openjdk exception conan-io/hooks#306

Merged

boussaffawalid force-pushed the openjdk branch from 7178919 to b567d90 Compare June 17, 2021 09:16

This comment has been minimized.

Sign in to view

boussaffawalid force-pushed the openjdk branch from b567d90 to 42738cb Compare June 17, 2021 09:35

This comment has been minimized.

Sign in to view

boussaffawalid force-pushed the openjdk branch from 42738cb to 180f4f9 Compare June 17, 2021 09:43

This comment has been minimized.

Sign in to view

added recipe for openjdk

794c28d

boussaffawalid force-pushed the openjdk branch from 8c471d3 to 794c28d Compare June 17, 2021 09:50

This comment has been minimized.

Sign in to view

conan-center-bot added the Unexpected Error label Jun 17, 2021

This comment has been minimized.

Sign in to view

conan-center-bot removed Unexpected Error Failed labels Jun 17, 2021

prince-chrismc approved these changes Jun 18, 2021

View reviewed changes

github-actions bot mentioned this pull request Jun 18, 2021

Pull Requests Ready to be Reviewed prince-chrismc/conan-center-index-pending-review#1

Open

jgsogo reviewed Jun 18, 2021

View reviewed changes

prince-chrismc mentioned this pull request Jun 26, 2021

Update how_to_add_packages.md #5962

Merged

This comment has been minimized.

Sign in to view

conan-center-bot requested a review from uilianries June 30, 2021 12:00

ericriff reviewed Jul 21, 2021

View reviewed changes

added required_conan_version

74d890d

boussaffawalid dismissed madebr’s stale review via 74d890d July 27, 2021 20:06

This comment has been minimized.

Sign in to view

conan-center-bot requested a review from uilianries August 10, 2021 11:59

conan-center-bot requested a review from jgsogo August 19, 2021 11:57

conan-center-bot requested a review from memsharded August 26, 2021 12:00

prince-chrismc previously approved these changes Aug 28, 2021

View reviewed changes

madebr reviewed Aug 28, 2021

View reviewed changes

stale bot added the stale label Sep 27, 2021

jgsogo reviewed Sep 27, 2021

View reviewed changes

stale bot removed the stale label Sep 27, 2021

Update recipes/openjdk/all/test_package/conanfile.py

8c87412

Co-authored-by: Javier G. Sogo <jgsogo@gmail.com>

boussaffawalid dismissed prince-chrismc’s stale review via 8c87412 September 27, 2021 07:40

This comment has been minimized.

Sign in to view

conan-center-bot added the Failed label Sep 27, 2021

prince-chrismc reviewed Sep 27, 2021

View reviewed changes

recipes/openjdk/all/test_package/conanfile.py Outdated Show resolved Hide resolved

Update recipes/openjdk/all/test_package/conanfile.py

9e093ce

Co-authored-by: Chris Mc <prince.chrismc@gmail.com>

conan-center-bot removed the Failed label Sep 27, 2021

prince-chrismc approved these changes Sep 28, 2021

View reviewed changes

jgsogo approved these changes Sep 29, 2021

View reviewed changes

jgsogo mentioned this pull request Sep 29, 2021

[question] providing applications to cci? #5839

Closed

SSE4 approved these changes Sep 30, 2021

View reviewed changes

uilianries approved these changes Sep 30, 2021

View reviewed changes

conan-center-bot merged commit 0ef2071 into conan-io:master Sep 30, 2021

added recipe for openjdk #5792

added recipe for openjdk #5792

Conversation

boussaffawalid commented Jun 7, 2021

This comment has been minimized.

Choose a reason for hiding this comment

This comment has been minimized.

prince-chrismc commented Jun 7, 2021

prince-chrismc commented Jun 7, 2021

boussaffawalid commented Jun 7, 2021

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

jgsogo left a comment

Choose a reason for hiding this comment

boussaffawalid commented Jun 23, 2021

This comment has been minimized.

SSE4 commented Jun 30, 2021

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

madebr Jul 27, 2021 • edited Loading

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

This comment has been minimized.

prince-chrismc commented Aug 28, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stale bot commented Sep 27, 2021

jgsogo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

conan-center-bot commented Sep 27, 2021

jgsogo left a comment

Choose a reason for hiding this comment

madebr Jul 27, 2021 •

edited

Loading

prince-chrismc commented Aug 28, 2021 •

edited

Loading