You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@arrow.apache.org by Wes McKinney <we...@gmail.com> on 2018/06/22 09:21:37 UTC

Re: Build system discussion for Arrow (and Orc?)

hi Michael,

I wanted to rekindle this discussion so that we can resolve these
issues before Arrow 0.10.0 is released. Can you let us know what needs
to be changed or submit a PR? I have a PR coming in for ARROW-902
shortly (offline builds), and I think it would be a good idea to
enable all third party dependencies to use dynamic linking if the user
so desired.

Thanks,
Wes

On Tue, Apr 24, 2018 at 10:54 AM, Wes McKinney <we...@gmail.com> wrote:
> +1. Definitely do open JIRAs to describe the issue(s) you are having,
> even if you do not intend to patch them yourself. The ORC issue may
> need to be patched upstream in Apache ORC. We're happy to help spec
> out the work required to help make the builds / packaging less painful
>
> Thanks
> Wes
>
> On Wed, Apr 11, 2018 at 5:39 AM, Uwe L. Korn <uw...@xhochy.com> wrote:
>> Hello Michael,
>>
>>> 1. Is this a welcome change, or should we just carry patches locally?
>>
>> These changes would be very welcome. The current vendoring approach exists for all dependencies mostly to get have a smooth development experience. It is not meant for releases. The current approach for ORC is mainly in this form as there are things missing to get to a smooth, non-vendored released binary.
>>
>>> 2. Assuming change is welcome, what is the preferred method for submitting
>>> changes?  Github PR(s)?
>>
>> As mentioned above, they are very welcome. The preferred method is creating an issue in JIRA https://issues.apache.org/jira/projects/ARROW and then opening a PR on github. The name of the PR should be prefixed with the issue. e.g. `ARROW-XXX: [Python] Better ORC packaging`
>>
>> Uwe

Re: Build system discussion for Arrow (and Orc?)

Posted by Michael Sarahan <ms...@gmail.com>.
I believe the fix was in ORC - they greatly improved their CMake stuff
since I first tried it.  The brokenness was not necessarily in arrow, but
in arrow's vendoring of orc, with orc using -werror by default.  That is no
longer the default.

Anyway, all fixed now, and I think the CMake stuff is in much better shape
now, so thank you to all who have contributed to that.

On Tue, Jun 26, 2018 at 3:16 PM Wes McKinney <we...@gmail.com> wrote:

> Thanks Michael.
>
> How did you hit the -Werror issue? I'm looking at your patch
>
>
> https://github.com/AnacondaRecipes/arrow-cpp-feedstock/blob/master/recipe/0001-remove-Werror-for-compilation-on-mac-and-linux-32.patch
>
> but this should not be impacting you. The default warning level is
> PRODUCTION, -Werror is only being added for CHECKIN and EVERYTHING
> warning levels (which we use in CI)
>
> On Fri, Jun 22, 2018 at 10:15 AM, Michael Sarahan <ms...@gmail.com>
> wrote:
> > Thanks for the reminder. I only very recently was able to finish up my
> work
> > on our arrow package.
> >
> >
> https://github.com/AnacondaRecipes/arrow-cpp-feedstock/tree/master/recipe
> >
> > There are some bugs in the patches I've just noticed - ORC_VENDORED isn't
> > set properly, copy paste error.
> >
> > I'd also like to put the -werror behind some kind of flag so that either
> we
> > can disable it, or you can enable it. I'm open to your opinions on what
> the
> > default behavior should be. IMHO, disabled is better as a default, but
> you
> > should enable it for your local builds and for CI.
> >
> > I am traveling today, but will try to put up a PR sometime this
> afternoon.
> >
> > On Fri, Jun 22, 2018, 04:22 Wes McKinney <we...@gmail.com> wrote:
> >
> >> hi Michael,
> >>
> >> I wanted to rekindle this discussion so that we can resolve these
> >> issues before Arrow 0.10.0 is released. Can you let us know what needs
> >> to be changed or submit a PR? I have a PR coming in for ARROW-902
> >> shortly (offline builds), and I think it would be a good idea to
> >> enable all third party dependencies to use dynamic linking if the user
> >> so desired.
> >>
> >> Thanks,
> >> Wes
> >>
> >> On Tue, Apr 24, 2018 at 10:54 AM, Wes McKinney <we...@gmail.com>
> >> wrote:
> >> > +1. Definitely do open JIRAs to describe the issue(s) you are having,
> >> > even if you do not intend to patch them yourself. The ORC issue may
> >> > need to be patched upstream in Apache ORC. We're happy to help spec
> >> > out the work required to help make the builds / packaging less painful
> >> >
> >> > Thanks
> >> > Wes
> >> >
> >> > On Wed, Apr 11, 2018 at 5:39 AM, Uwe L. Korn <uw...@xhochy.com>
> wrote:
> >> >> Hello Michael,
> >> >>
> >> >>> 1. Is this a welcome change, or should we just carry patches
> locally?
> >> >>
> >> >> These changes would be very welcome. The current vendoring approach
> >> exists for all dependencies mostly to get have a smooth development
> >> experience. It is not meant for releases. The current approach for ORC
> is
> >> mainly in this form as there are things missing to get to a smooth,
> >> non-vendored released binary.
> >> >>
> >> >>> 2. Assuming change is welcome, what is the preferred method for
> >> submitting
> >> >>> changes?  Github PR(s)?
> >> >>
> >> >> As mentioned above, they are very welcome. The preferred method is
> >> creating an issue in JIRA https://issues.apache.org/jira/projects/ARROW
> >> and then opening a PR on github. The name of the PR should be prefixed
> with
> >> the issue. e.g. `ARROW-XXX: [Python] Better ORC packaging`
> >> >>
> >> >> Uwe
> >>
>

Re: Build system discussion for Arrow (and Orc?)

Posted by Wes McKinney <we...@gmail.com>.
Thanks Michael.

How did you hit the -Werror issue? I'm looking at your patch

https://github.com/AnacondaRecipes/arrow-cpp-feedstock/blob/master/recipe/0001-remove-Werror-for-compilation-on-mac-and-linux-32.patch

but this should not be impacting you. The default warning level is
PRODUCTION, -Werror is only being added for CHECKIN and EVERYTHING
warning levels (which we use in CI)

On Fri, Jun 22, 2018 at 10:15 AM, Michael Sarahan <ms...@gmail.com> wrote:
> Thanks for the reminder. I only very recently was able to finish up my work
> on our arrow package.
>
> https://github.com/AnacondaRecipes/arrow-cpp-feedstock/tree/master/recipe
>
> There are some bugs in the patches I've just noticed - ORC_VENDORED isn't
> set properly, copy paste error.
>
> I'd also like to put the -werror behind some kind of flag so that either we
> can disable it, or you can enable it. I'm open to your opinions on what the
> default behavior should be. IMHO, disabled is better as a default, but you
> should enable it for your local builds and for CI.
>
> I am traveling today, but will try to put up a PR sometime this afternoon.
>
> On Fri, Jun 22, 2018, 04:22 Wes McKinney <we...@gmail.com> wrote:
>
>> hi Michael,
>>
>> I wanted to rekindle this discussion so that we can resolve these
>> issues before Arrow 0.10.0 is released. Can you let us know what needs
>> to be changed or submit a PR? I have a PR coming in for ARROW-902
>> shortly (offline builds), and I think it would be a good idea to
>> enable all third party dependencies to use dynamic linking if the user
>> so desired.
>>
>> Thanks,
>> Wes
>>
>> On Tue, Apr 24, 2018 at 10:54 AM, Wes McKinney <we...@gmail.com>
>> wrote:
>> > +1. Definitely do open JIRAs to describe the issue(s) you are having,
>> > even if you do not intend to patch them yourself. The ORC issue may
>> > need to be patched upstream in Apache ORC. We're happy to help spec
>> > out the work required to help make the builds / packaging less painful
>> >
>> > Thanks
>> > Wes
>> >
>> > On Wed, Apr 11, 2018 at 5:39 AM, Uwe L. Korn <uw...@xhochy.com> wrote:
>> >> Hello Michael,
>> >>
>> >>> 1. Is this a welcome change, or should we just carry patches locally?
>> >>
>> >> These changes would be very welcome. The current vendoring approach
>> exists for all dependencies mostly to get have a smooth development
>> experience. It is not meant for releases. The current approach for ORC is
>> mainly in this form as there are things missing to get to a smooth,
>> non-vendored released binary.
>> >>
>> >>> 2. Assuming change is welcome, what is the preferred method for
>> submitting
>> >>> changes?  Github PR(s)?
>> >>
>> >> As mentioned above, they are very welcome. The preferred method is
>> creating an issue in JIRA https://issues.apache.org/jira/projects/ARROW
>> and then opening a PR on github. The name of the PR should be prefixed with
>> the issue. e.g. `ARROW-XXX: [Python] Better ORC packaging`
>> >>
>> >> Uwe
>>

Re: Build system discussion for Arrow (and Orc?)

Posted by Michael Sarahan <ms...@gmail.com>.
Thanks for the reminder. I only very recently was able to finish up my work
on our arrow package.

https://github.com/AnacondaRecipes/arrow-cpp-feedstock/tree/master/recipe

There are some bugs in the patches I've just noticed - ORC_VENDORED isn't
set properly, copy paste error.

I'd also like to put the -werror behind some kind of flag so that either we
can disable it, or you can enable it. I'm open to your opinions on what the
default behavior should be. IMHO, disabled is better as a default, but you
should enable it for your local builds and for CI.

I am traveling today, but will try to put up a PR sometime this afternoon.

On Fri, Jun 22, 2018, 04:22 Wes McKinney <we...@gmail.com> wrote:

> hi Michael,
>
> I wanted to rekindle this discussion so that we can resolve these
> issues before Arrow 0.10.0 is released. Can you let us know what needs
> to be changed or submit a PR? I have a PR coming in for ARROW-902
> shortly (offline builds), and I think it would be a good idea to
> enable all third party dependencies to use dynamic linking if the user
> so desired.
>
> Thanks,
> Wes
>
> On Tue, Apr 24, 2018 at 10:54 AM, Wes McKinney <we...@gmail.com>
> wrote:
> > +1. Definitely do open JIRAs to describe the issue(s) you are having,
> > even if you do not intend to patch them yourself. The ORC issue may
> > need to be patched upstream in Apache ORC. We're happy to help spec
> > out the work required to help make the builds / packaging less painful
> >
> > Thanks
> > Wes
> >
> > On Wed, Apr 11, 2018 at 5:39 AM, Uwe L. Korn <uw...@xhochy.com> wrote:
> >> Hello Michael,
> >>
> >>> 1. Is this a welcome change, or should we just carry patches locally?
> >>
> >> These changes would be very welcome. The current vendoring approach
> exists for all dependencies mostly to get have a smooth development
> experience. It is not meant for releases. The current approach for ORC is
> mainly in this form as there are things missing to get to a smooth,
> non-vendored released binary.
> >>
> >>> 2. Assuming change is welcome, what is the preferred method for
> submitting
> >>> changes?  Github PR(s)?
> >>
> >> As mentioned above, they are very welcome. The preferred method is
> creating an issue in JIRA https://issues.apache.org/jira/projects/ARROW
> and then opening a PR on github. The name of the PR should be prefixed with
> the issue. e.g. `ARROW-XXX: [Python] Better ORC packaging`
> >>
> >> Uwe
>