Skip to content

Conversation

@ztzg
Copy link
Contributor

@ztzg ztzg commented Dec 9, 2019

ZOOKEEPER-3635 changed the versioning scheme for the C client from integer-valued ZOO_{MAJOR,MINOR,PATCH}_VERSION definitions to a single string-valued #define ZOO_VERSION "3.6.0".

This caused the Perl and Python contribs to fail to build. The attached patches should repair the issue.

@eolivelli, @nkalmar: What do you think?

@asf-ci
Copy link

asf-ci commented Dec 9, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build-maven/1698/

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

My bad!
I did not check the contrib modules!

Let's commit this fix and fix master branch.

Is there any way to create a script (or use Maven) to build these modules in Travis?

It would be good to move this 'official' clients to the zookeeper-clients module and move them out of 'contrib'

I thought they were quite unused but id there is interested we should bring them to the front row together with the C client

@ztzg
Copy link
Contributor Author

ztzg commented Dec 9, 2019

Hi @eolivelli,

(Github tells me you "requested changes," but I cannot find review notes. Is it only about the questions below, or am I missing something?)

My bad! I did not check the contrib modules!

Right; this is bound to happen if they are not part of the release checklist.

Let's commit this fix and fix master branch.

Okay :)

Is there any way to create a script (or use Maven) to build these modules in Travis?

I suppose this is the "change" you are requesting? I can have a look, but should such a thing not be done as another PR/ticket? (I'm not familiar with the plumbing of the Apache/Maven CI, and have no idea of how many iterations it will require.)

It would be good to move this 'official' clients to the zookeeper-clients module and move them out of 'contrib'
I thought they were quite unused but id there is interested we should bring them to the front row together with the C client

I am aware of some users of the "official" Perl client. I don't know any users of the "official" Python client; most seem to be using Kazoo instead.

@symat
Copy link
Contributor

symat commented Dec 9, 2019

Is there any way to create a script (or use Maven) to build these modules in Travis?

in #1121 I did some changes to be able to build zkpython at least with ant (it broke after the ant removal). I think it would be relatively easy to implement the maven-based build jobs as well (at least for zkpython). Although I am not sure how long do we want to keep these contrib projects in the main ZooKeeper repo.

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

Sorry, I meant to click on "Approve"

@eolivelli
Copy link
Contributor

@nkalmar PTAL and merge if you approve this patch

@ztzg
Copy link
Contributor Author

ztzg commented Dec 9, 2019

Is there any way to create a script (or use Maven) to build these modules in Travis?

in #1121 I did some changes to be able to build zkpython at least with ant (it broke after the ant removal). I think it would be relatively easy to implement the maven-based build jobs as well (at least for zkpython).

Right. I can cargo-cult Maven stanzas, but I don't have the slightest idea of how to get "native" dependencies installed on build hosts at this point.

I see this in the build log:

checking for perl... /usr/bin/perl

and I suppose Python is there too—but I don't know if development headers are available. I guess I could "just try." But given a build host such as Agent H14, would you know if it is possible to retrieve a manifest of installed packages?

Although I am not sure how long do we want to keep these contrib projects in the main ZooKeeper repo.

Note that @eolivelli was suggesting to promote them from zookeeper-contrib/* to zookeeper-client/*, not to get rid of them :)

@eolivelli
Copy link
Contributor

It is better to use Travis.
With travis you can install your dependencies, like we do for cppunit

@ztzg
Copy link
Contributor Author

ztzg commented Dec 9, 2019

Okay—noted!

Copy link
Contributor

@nkalmar nkalmar left a comment

Choose a reason for hiding this comment

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

Nice catch, +1

@asfgit asfgit closed this in bcd6d3b Dec 10, 2019
@nkalmar
Copy link
Contributor

nkalmar commented Dec 10, 2019

Thanks @ztzg , merged to master (3.6.0)

The travis integration can be done in a separate Jira.

junyoungKimGit pushed a commit to junyoungKimGit/zookeeper that referenced this pull request Feb 7, 2020
[ZOOKEEPER-3635](https://issues.apache.org/jira/browse/ZOOKEEPER-3635) changed the versioning scheme for the C client from integer-valued `ZOO_{MAJOR,MINOR,PATCH}_VERSION` definitions to a single string-valued `#define ZOO_VERSION "3.6.0"`.

This caused the Perl and Python contribs to fail to build.  The attached patches should repair the issue.

eolivelli, nkalmar: What do you think?

Author: Damien Diederen <dd@crosstwine.com>

Reviewers: Enrico Olivelli <eolivelli@apache.org>, Norbert Kalmar <nkalmar@apache.org>

Closes apache#1169 from ztzg/ZOOKEEPER-3641-fix-zoo-version-contribs
stickyhipp pushed a commit to stickyhipp/zookeeper that referenced this pull request Aug 19, 2020
[ZOOKEEPER-3635](https://issues.apache.org/jira/browse/ZOOKEEPER-3635) changed the versioning scheme for the C client from integer-valued `ZOO_{MAJOR,MINOR,PATCH}_VERSION` definitions to a single string-valued `#define ZOO_VERSION "3.6.0"`.

This caused the Perl and Python contribs to fail to build.  The attached patches should repair the issue.

eolivelli, nkalmar: What do you think?

Author: Damien Diederen <dd@crosstwine.com>

Reviewers: Enrico Olivelli <eolivelli@apache.org>, Norbert Kalmar <nkalmar@apache.org>

Closes apache#1169 from ztzg/ZOOKEEPER-3641-fix-zoo-version-contribs
RokLenarcic pushed a commit to RokLenarcic/zookeeper that referenced this pull request Aug 31, 2022
[ZOOKEEPER-3635](https://issues.apache.org/jira/browse/ZOOKEEPER-3635) changed the versioning scheme for the C client from integer-valued `ZOO_{MAJOR,MINOR,PATCH}_VERSION` definitions to a single string-valued `#define ZOO_VERSION "3.6.0"`.

This caused the Perl and Python contribs to fail to build.  The attached patches should repair the issue.

eolivelli, nkalmar: What do you think?

Author: Damien Diederen <dd@crosstwine.com>

Reviewers: Enrico Olivelli <eolivelli@apache.org>, Norbert Kalmar <nkalmar@apache.org>

Closes apache#1169 from ztzg/ZOOKEEPER-3641-fix-zoo-version-contribs
RokLenarcic pushed a commit to RokLenarcic/zookeeper that referenced this pull request Sep 3, 2022
[ZOOKEEPER-3635](https://issues.apache.org/jira/browse/ZOOKEEPER-3635) changed the versioning scheme for the C client from integer-valued `ZOO_{MAJOR,MINOR,PATCH}_VERSION` definitions to a single string-valued `#define ZOO_VERSION "3.6.0"`.

This caused the Perl and Python contribs to fail to build.  The attached patches should repair the issue.

eolivelli, nkalmar: What do you think?

Author: Damien Diederen <dd@crosstwine.com>

Reviewers: Enrico Olivelli <eolivelli@apache.org>, Norbert Kalmar <nkalmar@apache.org>

Closes apache#1169 from ztzg/ZOOKEEPER-3641-fix-zoo-version-contribs
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.

5 participants