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) <avertl...@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), < > avertl...@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, <anto...@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) < > > >>> avertl...@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 > > >>>> <r...@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) > > >> < > > >>>>> avertl...@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) > > >>>> < > > >>>>>> avertl...@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 | jam...@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. > > >>>> > > >>>> > > >>>> > > >>> > > >> > > > > > > > > > > >
