You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Zhitao Li <zh...@gmail.com> on 2017/05/27 01:10:35 UTC
Review Request 59616: Add `six` python package in install_require.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59616/
-----------------------------------------------------------
Review request for mesos and Anand Mazumdar.
Repository: mesos
Description
-------
After protobuf 3.3.0 upgrade, reviewboard CI breaks complaining failed
`import six` python.
Diffs
-----
src/python/setup.py.in 991ac31409c566e8a59d47f8296317e245925d4e
Diff: https://reviews.apache.org/r/59616/diff/1/
Testing
-------
Thanks,
Zhitao Li
Re: Review Request 59616: Add `six` python package in install_require.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59616/#review176262
-----------------------------------------------------------
This looks like the wrong fix to me. Mesos does not depend on `six` but our own dependency `protobuf` does. I am not sure exactly what breakage this patch addresses, but we should probably fix the way we pull in the Python protobuf package into our build instead of fixing its broken dependencies by declaring incorrect deps.
Note there is also https://github.com/Homebrew/homebrew-core/pull/13876#issuecomment-303682598. We should probably let them know about what release any solutions found here landed in.
- Benjamin Bannier
On May 27, 2017, 3:10 a.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59616/
> -----------------------------------------------------------
>
> (Updated May 27, 2017, 3:10 a.m.)
>
>
> Review request for mesos and Anand Mazumdar.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> After protobuf 3.3.0 upgrade, reviewboard CI breaks complaining failed
> `import six` python.
>
>
> Diffs
> -----
>
> src/python/setup.py.in 991ac31409c566e8a59d47f8296317e245925d4e
>
>
> Diff: https://reviews.apache.org/r/59616/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 59616: Add `six` python package in install_require.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59616/#review176287
-----------------------------------------------------------
Patch looks great!
Reviews applied: [59616]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On May 27, 2017, 1:10 a.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59616/
> -----------------------------------------------------------
>
> (Updated May 27, 2017, 1:10 a.m.)
>
>
> Review request for mesos and Anand Mazumdar.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> After protobuf 3.3.0 upgrade, reviewboard CI breaks complaining failed
> `import six` python.
>
>
> Diffs
> -----
>
> src/python/setup.py.in 991ac31409c566e8a59d47f8296317e245925d4e
>
>
> Diff: https://reviews.apache.org/r/59616/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 59616: Add `six` python package in install_require.
Posted by Zhitao Li <zh...@gmail.com>.
> On May 29, 2017, 9:41 p.m., Kevin Klues wrote:
> > I agree with @benjamin. This is the improper fix for this problem. The underlying issue is that our build environment now requires `six` since protobuf-3.3.3 depends on it (the `mesos` package doesn't depend on it, just the build environment). If we were to install protobuf-3.3.3 with a package manage, such as `pip`, `apt-get`, or `yum`, we wouldn't have this problem. However, we bundle protobuf-3.3.3 into 3rdparty and just improt it directly (without bundling any of its dependencies). As such, we need to make sure and have any of its dependencies either pre-installed in our build environment, or bundle them in 3rdparty as well.
> >
> > See https://reviews.apache.org/r/59632/ for an alternative fix that gets `six` preinstalled in the build environment instead of bundling it directly.
@klueska/@benjamin, thanks for the feedback. This is just a quick try to see whether this can fix the CI.
A quick git grep reveals this line (https://github.com/google/protobuf/blob/83b0cc9ba6bd2f1a104f5e3d2d7b471b8fab1001/python/setup.py#L223) in protobuf code so I guess people who installs protobuf through `pip` or related tools should be covered, so I will abandon this review request since CI is fixed.
- Zhitao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59616/#review176274
-----------------------------------------------------------
On May 27, 2017, 1:10 a.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59616/
> -----------------------------------------------------------
>
> (Updated May 27, 2017, 1:10 a.m.)
>
>
> Review request for mesos and Anand Mazumdar.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> After protobuf 3.3.0 upgrade, reviewboard CI breaks complaining failed
> `import six` python.
>
>
> Diffs
> -----
>
> src/python/setup.py.in 991ac31409c566e8a59d47f8296317e245925d4e
>
>
> Diff: https://reviews.apache.org/r/59616/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 59616: Add `six` python package in install_require.
Posted by Kevin Klues <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59616/#review176274
-----------------------------------------------------------
I agree with @benjamin. This is the improper fix for this problem. The underlying issue is that our build environment now requires `six` since protobuf-3.3.3 depends on it (the `mesos` package doesn't depend on it, just the build environment). If we were to install protobuf-3.3.3 with a package manage, such as `pip`, `apt-get`, or `yum`, we wouldn't have this problem. However, we bundle protobuf-3.3.3 into 3rdparty and just improt it directly (without bundling any of its dependencies). As such, we need to make sure and have any of its dependencies either pre-installed in our build environment, or bundle them in 3rdparty as well.
See https://reviews.apache.org/r/59632/ for an alternative fix that gets `six` preinstalled in the build environment instead of bundling it directly.
- Kevin Klues
On May 27, 2017, 1:10 a.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59616/
> -----------------------------------------------------------
>
> (Updated May 27, 2017, 1:10 a.m.)
>
>
> Review request for mesos and Anand Mazumdar.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> After protobuf 3.3.0 upgrade, reviewboard CI breaks complaining failed
> `import six` python.
>
>
> Diffs
> -----
>
> src/python/setup.py.in 991ac31409c566e8a59d47f8296317e245925d4e
>
>
> Diff: https://reviews.apache.org/r/59616/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>