You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@arrow.apache.org by James Duong <ja...@bitquilltech.com.INVALID> on 2022/08/02 20:11:00 UTC

Re: Re: Help needed with PR #13659: Fixing build/unit test issues in msvc/win32

A heads-up, the warnings are now fixed in the 32-bit build n this PR:
https://github.com/apache/arrow/pull/13532

I haven't fixed the linker error yet -- disabling parquet build didn't seem
to work (or I haven't properly disabled the parquet build)
and need to fix Archery warnings.

On Tue, Jul 26, 2022 at 6:06 AM Arkadiy Vertleyb (BLOOMBERG/ 120 PARK) <
avertleyb@bloomberg.net> wrote:

> From: Arkadiy Vertleyb (BLOOMBERG/ 120 PARK) At: 07/26/22 08:53:54
> UTC-4:00To:  wesmckinn@gmail.com
> Subject: Re: Help needed with PR #13659: Fixing build/unit test issues in
> msvc/win32
> It's not just warnings.  Also fixing compile and unit test errors for 32
> bit MSVC:
>
> https://issues.apache.org/jira/browse/ARROW-16778
> https://github.com/apache/arrow/pull/13659
>
> The PR is currently failing some checks, mostly because of some unrelated
> issue with protobuf version 2.  The explanation can be found in the Raul's
> message on this subject.  There are also some formatting issues that I will
> fix when everything else is resolved.
>
> Regards,
> Arkadiy
>
> From: wesmckinn@gmail.com At: 07/25/22 17:13:44 UTC-4:00To:  Arkadiy
> Vertleyb (BLOOMBERG/ 120 PARK ) ,  dev@arrow.apache.org
> Subject: Re: Help needed with PR #13659: Fixing build/unit test issues in
> msvc/win32
>
> Suppressing the warnings on 32-bit MSVC sounds like a reasonable
> compromise. Is there an open PR for this (and what is the
> corresponding Jira issue so we don't lose track of it)?
>
> On Fri, Jul 22, 2022 at 1:23 PM Arkadiy Vertleyb (BLOOMBERG/ 120 PARK)
> <av...@bloomberg.net> wrote:
> >
> > Or live with the warnings.  Or cast his sizes to _int64, if the warnings
> are
> critical.
> >
> > From: dev@arrow.apache.org At: 07/22/22 14:06:34 UTC-4:00To:  Arkadiy
> Vertleyb (BLOOMBERG/ 120 PARK ) ,  dev@arrow.apache.org
> > Subject: Re: Help needed with PR #13659: Fixing build/unit test issues
> in
> msvc/win32
> >
> > The problem with (2) is that a library user can only work around it by
> > suppressing warnings with pragmas when including Arrow headers.
> >
> > This is somewhat unfriendly from a user perspective.
> >
> > On Fri., Jul. 22, 2022, 08:11 Arkadiy Vertleyb (BLOOMBERG/ 120 PARK), <
> > avertleyb@bloomberg.net> wrote:
> >
> > > This is what I would do:
> > >
> > > 1) Suppress warnings in cmake input file for this particular platform.
> > > This will let the user build the arrow libs.
> > >
> > > 2) Let those users who care about 32 bits worry about the warnings in
> the
> > > headers.
> > >
> > > Regards,
> > > Arkadiy
> > >
> > > From: dev@arrow.apache.org At: 07/22/22 10:18:09 UTC-4:00To:
> > > dev@arrow.apache.org
> > > Subject: Re: Help needed with PR #13659: Fixing build/unit test issues
> in
> > > msvc/win32
> > >
> > >
> > > Ah, that's a good point. Then we should probably use explicit casts.
> > >
> > >
> > > Le 22/07/2022 à 15:58, James Duong a écrit :
> > > > Seems reasonable to suppress this warning on this single platform.
> > > However
> > > > if we do this using a compiler option, we may hide warnings in public
> > > > headers from the build. This isn't great since library users may have
> > > > policies that disallow warnings.
> > > >
> > > > On Fri., Jul. 22, 2022, 05:47 Antoine Pitrou, <an...@python.org>
> > > wrote:
> > > >
> > > >>
> > > >> We could perhaps suppress the integer downcast warnings, but only on
> > > >> 32-bit Windows (not 64-bit, not other platforms).
> > > >>
> > > >> Regards
> > > >>
> > > >> Antoine.
> > > >>
> > > >>
> > > >> Le 22/07/2022 à 14:42, Arkadiy Vertleyb (BLOOMBERG/ 120 PARK) a
> écrit :
> > > >>> Hi James.
> > > >>>
> > > >>> I don't have strong feelings about whose PR is used and how
> exactly the
> > > >> issue is fixed.  All I care about at this point is working (and
> > > maintained)
> > > >> MSVC 32 bit version.  I need this for my project at work.
> > > >>>
> > > >>> If you feel that your PR will solve this issue, and is close to
> being
> > > >> approved, please let me know and I will stop worrying about this.
> > > >>>
> > > >>> If you PR is not being approved any time soon, I will proceed with
> > > mine,
> > > >> and you can add your changes on top of it.  Should not be a big
> deal,
> > > since
> > > >> I only make a few small changes.
> > > >>>
> > > >>> Please let me know how you think we should proceed with this.
> > > >>>
> > > >>> Thanks,
> > > >>> Arkadiy
> > > >>>
> > > >>> From: dev@arrow.apache.org At: 07/21/22 18:29:33 UTC-4:00To:
> Arkadiy
> > > >> Vertleyb (BLOOMBERG/ 120 PARK ) ,  dev@arrow.apache.org
> > > >>> Subject: Re: Help needed with PR #13659: Fixing build/unit test
> issues
> > > >> in msvc/win32
> > > >>>
> > > >>> Feedback I got here was to use static_cast:
> > > >>> https://github.com/apache/arrow/pull/13532#issuecomment-1177488433
> > > >>>
> > > >>> I'm indifferent as to whether we want to do the static_casts or
> just
> > > >>> suppress the warning as you've done.
> > > >>>
> > > >>> Your PR isn't building the 32-bit build in CI btw. It fails finding
> > > >> OpenSSL:
> > > >>>
> > > >>
> > >
> > >
> >
>
> https://github.com/apache/arrow/runs/7432759888?check_suite_focus=true#step:7:20
> > > >>> 1
> > > >>>
> > > >>> I've fixed this in the changes to the github workflow in my PR.
> > > >>>
> > > >>>
> > > >>> On Thu, Jul 21, 2022 at 12:47 PM Arkadiy Vertleyb (BLOOMBERG/ 120
> > > PARK) <
> > > >>> avertleyb@bloomberg.net> wrote:
> > > >>>
> > > >>>> Hi James.
> > > >>>>
> > > >>>> My PR makes the compiler ignore the warnings.
> > > >>>>
> > > >>>> As far as I understand, this issue cannot be consistently resolved
> > > >> within
> > > >>>> the Google paradigm arrow follows on this subject.  The google
> > > paradigm
> > > >>>> requires to treat all the sizes as signed 64 bit integers,
> regardless
> > > of
> > > >>>> the architecture.  This paradigm is obviously at odds with the
> > > standard
> > > >> C++
> > > >>>> paradigm.
> > > >>>>
> > > >>>> Changing of the paradigm is obviously not anything I want to
> propose
> > > at
> > > >>>> this point, hence I don't see any other way as to just switch off
> the
> > > >>>> warnings.
> > > >>>>
> > > >>>> Thanks,
> > > >>>> Arkadiy
> > > >>>>
> > > >>>>
> > > >>>> From: dev@arrow.apache.org At: 07/21/22 13:59:51 UTC-4:00To:
> > > >>>> dev@arrow.apache.org
> > > >>>> Cc:  Arkadiy Vertleyb (BLOOMBERG/ 120 PARK )
> > > >>>> Subject: Re: Help needed with PR #13659: Fixing build/unit test
> issues
> > > >> in
> > > >>>> msvc/win32
> > > >>>>
> > > >>>> Hi Arkadiy,
> > > >>>>
> > > >>>> I've been working on a PR for fixing 32-bit Visual Studio here
> which
> > > has
> > > >>>> some of the same changes.
> > > >>>> It also enables the 32-bit VS build in CI, which fails due to a
> ton of
> > > >>>> integer implicit cast warnings:
> > > >>>> https://github.com/apache/arrow/pull/13532
> > > >>>>
> > > >>>> Most of this commit is fixing 32-bit cast errors, along with a few
> > > >> changes
> > > >>>> to call bit_util::PopCount instead of ARROW_POPCOUNT64()
> > > >>>>
> > > >>>> On Thu, Jul 21, 2022 at 7:22 AM Raul Cumplido Dominguez
> > > >>>> <ra...@voltrondata.com.invalid> wrote:
> > > >>>>
> > > >>>>> Yes, issues 1-3 are not related to your PR.
> > > >>>>>
> > > >>>>> On Thu, Jul 21, 2022 at 4:04 PM Arkadiy Vertleyb (BLOOMBERG/ 120
> > > PARK)
> > > >> <
> > > >>>>> avertleyb@bloomberg.net> wrote:
> > > >>>>>
> > > >>>>>> Thanks Raul.
> > > >>>>>>
> > > >>>>>> Does this mean issues 1-3 are not really caused by my PR and I
> just
> > > >>>> need
> > > >>>>>> to wait for them to be fixed?
> > > >>>>>>
> > > >>>>>>
> > > >>>>>> From: dev@arrow.apache.org At: 07/21/22 09:51:09 UTC-4:00To:
> > > Arkadiy
> > > >>>>>> Vertleyb (BLOOMBERG/ 120 PARK ) ,  dev@arrow.apache.org
> > > >>>>>> Subject: Re: Help needed with PR #13659: Fixing build/unit test
> > > issues
> > > >>>> in
> > > >>>>>> msvc/win32
> > > >>>>>>
> > > >>>>>> Hi Arkadiy,
> > > >>>>>>
> > > >>>>>> For issues 2 and 3 there is currently an issue [1] with the
> protobuf
> > > >>>>>> version [2] distributed with homebrew [3] happening on master.
> These
> > > >>>> ones
> > > >>>>>> should be fixed once the upstream homebrew package is
> distributed.
> > > >>>>>> Issue 1 is also happening on master and I am not sure whether
> the
> > > >> issue
> > > >>>>> is
> > > >>>>>> tracked independently but there was a fix [4] on a PR [5]. I'll
> > > follow
> > > >>>>> that
> > > >>>>>> one up.
> > > >>>>>>
> > > >>>>>> Thanks,
> > > >>>>>> Raúl
> > > >>>>>>
> > > >>>>>> [1] https://issues.apache.org/jira/browse/ARROW-17162
> > > >>>>>> [2] https://github.com/protocolbuffers/protobuf/pull/10271
> > > >>>>>> [3] https://github.com/Homebrew/homebrew-core/pull/106252
> > > >>>>>> [4]
> > > >>>>>>
> > > >>>>>>
> > > >>>>>
> > > >>>>
> > > >>>>
> > > >>>
> > > >>
> > >
> > >
> >
>
> https://github.com/apache/arrow/pull/13634/commits/9e10f6c3399d83ebce5af551561fa
> > > >>>>>> 3a16da9cd5e
> > > >>>>>> <
> > > >>>>>
> > > >>>>
> > > >>>>
> > > >>>
> > > >>
> > >
> > >
> >
>
> https://github.com/apache/arrow/pull/13634/commits/9e10f6c3399d83ebce5af551561fa
> > > >>>> 3a16da9cd5e
> > > >>>>
> > > >>> <
> > > >>
> > >
> > >
> >
>
> https://github.com/apache/arrow/pull/13634/commits/9e10f6c3399d83ebce5af551561fa
> > > 3a16da9cd5e
> > >
> >
> <
> https://github.com/apache/arrow/pull/13634/commits/9e10f6c3399d83ebce5af551561fa3a16da9cd5e
> >
> > > >>>
> > > >>>>>>
> > > >>>>>> [5] https://github.com/apache/arrow/pull/13634
> > > >>>>>>
> > > >>>>>> On Thu, Jul 21, 2022 at 3:24 PM Arkadiy Vertleyb (BLOOMBERG/ 120
> > > PARK)
> > > >>>> <
> > > >>>>>> avertleyb@bloomberg.net> wrote:
> > > >>>>>>
> > > >>>>>>> Hi all.
> > > >>>>>>>
> > > >>>>>>> Can someone help me understand how the changes in this PR (
> > > >>>>>>>
> > > >>>>>>
> > > >>>>>>
> > > >>>>>
> > > >>>>
> > > >>>>
> > > >>>
> > > >>
> > >
> > >
> >
>
> https://github.com/apache/arrow/pull/13659/commits/e77ec9a84dab750bf016f9f5bd02e
> > > >>>>>> a48f2c8d77f
> > > >>>>>> <
> > > >>>>>
> > > >>>>
> > > >>>>
> > > >>>
> > > >>
> > >
> > >
> >
>
> https://github.com/apache/arrow/pull/13659/commits/e77ec9a84dab750bf016f9f5bd02e
> > > >>>> a48f2c8d77f
> > > >>>>
> > > >>> <
> > > >>
> > >
> > >
> >
>
> https://github.com/apache/arrow/pull/13659/commits/e77ec9a84dab750bf016f9f5bd02e
> > > a48f2c8d77f
> > >
> >
> <
> https://github.com/apache/arrow/pull/13659/commits/e77ec9a84dab750bf016f9f5bd02ea48f2c8d77f
> >
> > > >>>
> > > >>>>>>
> > > >>>>>> )
> > > >>>>>>> caused the following build failures?
> > > >>>>>>>
> > > >>>>>>> Thanks,
> > > >>>>>>> Arkadiy
> > > >>>>>>>
> > > >>>>>>> Here are the failures:
> > > >>>>>>>
> > > >>>>>>> 1) AMD64 MacOS 10.15 GLib & Ruby
> > > >>>>>>>
> > > >>>>>>> c_glib/arrow-glib/meson.build:216:0: ERROR: Program
> 'glib-mkenums
> > > >>>>>> mkenums'
> > > >>>>>>> not found or not executable
> > > >>>>>>>
> > > >>>>>>> 2) AMD64 MacOS 10.15 Python 3
> > > >>>>>>>
> > > >>>>>>> E   ImportError:
> > > >>>> dlopen(/usr/local/lib/python3.9/site-packages/pyarrow/
> > > >>>>>>> lib.cpython-39-darwin.so, 2): Symbol not found:
> > > >>>>>>> __ZN6google8protobuf8internal16InternalMetadataD1Ev
> > > >>>>>>> E     Referenced from: /usr/local/lib/libarrow.900.dylib
> > > >>>>>>> E     Expected in: flat namespace
> > > >>>>>>> E    in /usr/local/lib/libarrow.900.dylib
> > > >>>>>>>
> > > >>>>>>> 3) AMD64 MacOS 10.15 C++
> > > >>>>>>>
> > > >>>>>>> Undefined symbols for architecture x86_64:
> > > >>>>>>>
> > > >>>>
> > > "google::protobuf::internal::InternalMetadata::~InternalMetadata()",
> > > >>>>>>> referenced from:
> > > >>>>>>>         google::protobuf::MessageLite::~MessageLite() in
> > > >>>>>>> libopentelemetry_proto.a(trace_service.pb.cc.o)
> > > >>>>>>>         google::protobuf::MessageLite::~MessageLite() in
> > > >>>>>>> libopentelemetry_proto.a(trace.pb.cc.o)
> > > >>>>>>>         google::protobuf::MessageLite::~MessageLite() in
> > > >>>>>>> libopentelemetry_proto.a(common.pb.cc.o)
> > > >>>>>>>         google::protobuf::MessageLite::~MessageLite() in
> > > >>>>>>> libopentelemetry_proto.a(resource.pb.cc.o)
> > > >>>>>>> ld: symbol(s) not found for architecture x86_64
> > > >>>>>>>
> > > >>>>>>> 4) AMD64 Windows 2019 Win32 C++17
> > > >>>>>>>
> > > >>>>>>> -- Could NOT find SnappyAlt (missing: Snappy_LIB
> > > Snappy_INCLUDE_DIR)
> > > >>>>>>> -- Building snappy from source
> > > >>>>>>> CMake Error at C:/Program
> > > >>>>>>>
> > > >>>>>>
> > > >>>>>
> > > >>>>
> > > >>
> > >
> Files/CMake/share/cmake-3.23/Modules/FindPackageHandleStandardArgs.cmake:230
> > > >>>>>>> (message):
> > > >>>>>>>     Could NOT find OpenSSL, try to set the path to OpenSSL root
> > > folder
> > > >>>> in
> > > >>>>>> the
> > > >>>>>>>     system variable OPENSSL_ROOT_DIR (missing:
> > > OPENSSL_CRYPTO_LIBRARY)
> > > >>>>>> (found
> > > >>>>>>>     suitable version "1.1.1i", minimum required is "1.0.2")
> > > >>>>>>> Call Stack (most recent call first):
> > > >>>>>>>     C:/Program
> > > >>>>>>>
> > > >>>>>>
> > > >>>>>
> > > >>>>
> > > >>
> > >
> Files/CMake/share/cmake-3.23/Modules/FindPackageHandleStandardArgs.cmake:594
> > > >>>>>>> (_FPHSA_FAILURE_MESSAGE)
> > > >>>>>>>     C:/Program
> > > >>>> Files/CMake/share/cmake-3.23/Modules/FindOpenSSL.cmake:578
> > > >>>>>>> (find_package_handle_standard_args)
> > > >>>>>>>     cmake_modules/ThirdpartyToolchain.cmake:1253 (find_package)
> > > >>>>>>>     CMakeLists.txt:575 (include)
> > > >>>>>>>
> > > >>>>>>>
> > > >>>>>>>
> > > >>>>>>
> > > >>>>>>
> > > >>>>>>
> > > >>>>>
> > > >>>>
> > > >>>>
> > > >>>> --
> > > >>>>
> > > >>>> *James Duong*
> > > >>>> Lead Software Developer
> > > >>>> Bit Quill Technologies Inc.
> > > >>>> Direct: +1.604.562.6082 | jamesd@bitquilltech.com
> > > >>>> https://www.bitquilltech.com
> > > >>>>
> > > >>>> This email message is for the sole use of the intended
> recipient(s)
> > > and
> > > >> may
> > > >>>> contain confidential and privileged information.  Any unauthorized
> > > >> review,
> > > >>>> use, disclosure, or distribution is prohibited.  If you are not
> the
> > > >>>> intended recipient, please contact the sender by reply email and
> > > destroy
> > > >>>> all copies of the original message.  Thank you.
> > > >>>>
> > > >>>>
> > > >>>>
> > > >>>
> > > >>
> > > >
> > >
> > >
> > >
> >
> >
>
>
>

-- 

*James Duong*
Lead Software Developer
Bit Quill Technologies Inc.
Direct: +1.604.562.6082 | jamesd@bitquilltech.com
https://www.bitquilltech.com

This email message is for the sole use of the intended recipient(s) and may
contain confidential and privileged information.  Any unauthorized review,
use, disclosure, or distribution is prohibited.  If you are not the
intended recipient, please contact the sender by reply email and destroy
all copies of the original message.  Thank you.