Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Dependencies for Arrow's Azure Filesystem #1431

Merged
merged 4 commits into from
Jun 21, 2024

Conversation

srilman
Copy link
Contributor

@srilman srilman commented May 24, 2024

Resolved #1430

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@srilman srilman changed the title Add Dependencies for Azure Filesystems Add Dependencies for Arrow's Azure Filesystem May 24, 2024
@h-vetinari
Copy link
Member

h-vetinari commented May 25, 2024

Thanks for the PR! You can try -DCMAKE_UNITY_BUILD=OFF in bld.bat, that should at least give better error messages.

@srilman
Copy link
Contributor Author

srilman commented May 28, 2024

Main Error Message:

error C2027: use of undefined type 'arrow::fs::AzureFileSystem::Impl'

Here is the full error message:

FAILED: src/arrow/CMakeFiles/arrow_filesystem_shared.dir/filesystem/filesystem.cc.obj 
C:\PROGRA~1\MICROS~2\2022\ENTERP~1\VC\Tools\MSVC\1429~1.301\bin\HostX64\x64\cl.exe  /nologo /TP -DABSL_CONSUME_DLL -DARROW_EXPORTING -DARROW_HAVE_RUNTIME_AVX2 -DARROW_HAVE_RUNTIME_AVX512 -DARROW_HAVE_RUNTIME_BMI2 -DARROW_HAVE_RUNTIME_SSE4_2 -DARROW_WITH_TIMING_TESTS -DAWS_AUTH_USE_IMPORT_EXPORT -DAWS_CAL_USE_IMPORT_EXPORT -DAWS_CHECKSUMS_USE_IMPORT_EXPORT -DAWS_COMMON_USE_IMPORT_EXPORT -DAWS_COMPRESSION_USE_IMPORT_EXPORT -DAWS_CRT_CPP_USE_IMPORT_EXPORT -DAWS_EVENT_STREAM_USE_IMPORT_EXPORT -DAWS_HTTP_USE_IMPORT_EXPORT -DAWS_IO_USE_IMPORT_EXPORT -DAWS_MQTT_USE_IMPORT_EXPORT -DAWS_S3_USE_IMPORT_EXPORT -DAWS_SDKUTILS_USE_IMPORT_EXPORT -DAWS_SDK_VERSION_MAJOR=1 -DAWS_SDK_VERSION_MINOR=11 -DAWS_SDK_VERSION_PATCH=329 -DAWS_USE_IO_COMPLETION_PORTS -DAZ_CORE_DLL -DAZ_IDENTITY_DLL -DAZ_RTTI -DAZ_STORAGE_BLOBS_DLL -DAZ_STORAGE_COMMON_DLL -DAZ_STORAGE_FILES_DATALAKE_DLL -DPROTOBUF_USE_DLLS -DUSE_IMPORT_EXPORT -DUSE_IMPORT_EXPORT=1 -DUSE_WINDOWS_DLL_SEMANTICS -D_CRT_SECURE_NO_WARNINGS -D_ENABLE_EXTENDED_ALIGNED_STORAGE -I%SRC_DIR%\cpp\build\src -I%SRC_DIR%\cpp\src -I%SRC_DIR%\cpp\src\generated -external:I%PREFIX%\Library\include -external:I%SRC_DIR%\cpp\thirdparty\hadoop\include -external:W0 /DWIN32 /D_WINDOWS /GR /EHsc /D_SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING  /EHsc /wd5105 /bigobj /utf-8 /W3 /wd4800 /wd4996 /wd4065  /O2 /Ob2 /DNDEBUG -std:c++17 -MD /showIncludes /Fosrc\arrow\CMakeFiles\arrow_filesystem_shared.dir\filesystem\filesystem.cc.obj /Fdsrc\arrow\CMakeFiles\arrow_filesystem_shared.dir\ /FS -c %SRC_DIR%\cpp\src\arrow\filesystem\filesystem.cc
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.29.30133\include\memory(3089): error C2027: use of undefined type 'arrow::fs::AzureFileSystem::Impl'
%SRC_DIR%\cpp\src\arrow/filesystem/azurefs.h(232): note: see declaration of 'arrow::fs::AzureFileSystem::Impl'
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.29.30133\include\memory(3088): note: while compiling class template member function 'void std::default_delete<arrow::fs::AzureFileSystem::Impl>::operator ()(_Ty *) noexcept const'
        with
        [
            _Ty=arrow::fs::AzureFileSystem::Impl
        ]
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.29.30133\include\memory(3198): note: see reference to function template instantiation 'void std::default_delete<arrow::fs::AzureFileSystem::Impl>::operator ()(_Ty *) noexcept const' being compiled
        with
        [
            _Ty=arrow::fs::AzureFileSystem::Impl
        ]
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.29.30133\include\memory(3125): note: see reference to class template instantiation 'std::default_delete<arrow::fs::AzureFileSystem::Impl>' being compiled
%SRC_DIR%\cpp\src\arrow/filesystem/azurefs.h(233): note: see reference to class template instantiation 'std::unique_ptr<arrow::fs::AzureFileSystem::Impl,std::default_delete<arrow::fs::AzureFileSystem::Impl>>' being compiled
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.29.30133\include\memory(3089): error C2338: can't delete an incomplete type
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.29.30133\include\memory(3090): warning C4150: deletion of pointer to incomplete type 'arrow::fs::AzureFileSystem::Impl'; no destructor called
%SRC_DIR%\cpp\src\arrow/filesystem/azurefs.h(232): note: see declaration of 'arrow::fs::AzureFileSystem::Impl'

@srilman
Copy link
Contributor Author

srilman commented May 28, 2024

@h-vetinari I'm not super familiar with Windows builds / MSVC in general. Is there any chance I can disable ADLS on Windows and only include it for Mac and Linux, and make Windows a follow-up?

Copy link
Member

@h-vetinari h-vetinari left a comment

Choose a reason for hiding this comment

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

It's possible to exclude windows, but unless the urgency here is unusually high, I'd like to figure out why the windows builds are failing, and fix it. Could you raise an upstream issue with the error message here? It's possible to backport patches if/once they appear.

recipe/bld.bat Outdated Show resolved Hide resolved
recipe/meta.yaml Outdated
Comment on lines 62 to 66
- azure-core-cpp
- azure-identity-cpp
- azure-storage-blobs-cpp
- azure-storage-common-cpp
- azure-storage-files-datalake-cpp
Copy link
Member

@h-vetinari h-vetinari May 30, 2024

Choose a reason for hiding this comment

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

and restrict the dependencies here

Suggested change
- azure-core-cpp
- azure-identity-cpp
- azure-storage-blobs-cpp
- azure-storage-common-cpp
- azure-storage-files-datalake-cpp
- azure-core-cpp # [unix]
- azure-identity-cpp # [unix]
- azure-storage-blobs-cpp # [unix]
- azure-storage-common-cpp # [unix]
- azure-storage-files-datalake-cpp # [unix]

(same below)

@srilman
Copy link
Contributor Author

srilman commented May 30, 2024

For my use case, it is urgent yes, so I would prefer to do so if possible. I will make the changes you suggested as well as post this issue in the Arrow repo.

Copy link
Member

@h-vetinari h-vetinari left a comment

Choose a reason for hiding this comment

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

I've started picking your changes into #1435, but we're not ready to merge this until someone adds the azure-deps to the global pinning, and cleans up the azure-* feedstocks so they're all built for a consistent set of versions.

Comment on lines 5 to 12
azure_core_cpp:
- 1.11.1
azure_identity_cpp:
- 1.6.0
azure_storage_blobs_cpp:
- 12.10.0
azure_storage_common_cpp:
- 12.5.0
azure_storage_files_datalake_cpp:
- 12.9.0
Copy link
Member

Choose a reason for hiding this comment

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

You cannot set this manually, it needs to be done correctly: conda-forge/conda-forge-pinning-feedstock#6003

@srilman
Copy link
Contributor Author

srilman commented Jun 7, 2024

@h-vetinari To be honest, I don't really understand what you mean by global pinning. Is that something I could help with? Is there anything I can do about the azure dep versionings as well? As I said, would like to get this in ASAP

@h-vetinari
Copy link
Member

@h-vetinari To be honest, I don't really understand what you mean by global pinning.

It is what's under https://github.com/conda-forge/conda-forge-pinning-feedstock, specifically here.

Is that something I could help with? Is there anything I can do about the azure dep versionings as well? As I said, would like to get this in ASAP

You can help the maintainers of the azure-* feedstocks clean up the state of their feedstocks by helping with conda-forge/conda-forge-pinning-feedstock#6003.

Co-authored-by: H. Vetinari <h.vetinari@gmx.com>
@h-vetinari
Copy link
Member

Tried the migrator from conda-forge/conda-forge-pinning-feedstock#6056, but it's not ready yet (this is exactly why we need conda-forge/conda-forge-pinning-feedstock#6003)

Could not solve for environment specs
The following packages are incompatible
├─ azure-core-cpp 1.12.0.*  is requested and can be installed;
├─ azure-identity-cpp 1.8.0.*  is installable and it requires
│  └─ azure-core-cpp >=1.12.0,<1.12.1.0a0 , which can be installed;
├─ azure-storage-blobs-cpp 12.11.0.*  is not installable because it requires
│  └─ azure-core-cpp >=1.11.3,<1.11.4.0a0 , which conflicts with any installable versions previously reported;
└─ azure-storage-files-datalake-cpp 12.10.0.*  is not installable because it requires
   └─ azure-core-cpp >=1.11.3,<1.11.4.0a0 , which conflicts with any installable versions previously reported.

@jdblischak
Copy link
Member

The binaries for azure-storage-files-datalake-cpp are being built and uploaded now. If you restart the build in an hour or so, the conda solver error should be resolved.

Copy link
Member

@h-vetinari h-vetinari left a comment

Choose a reason for hiding this comment

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

Thanks for your patience

@h-vetinari h-vetinari added the automerge Merge the PR when CI passes label Jun 21, 2024
@github-actions github-actions bot merged commit 5016d21 into conda-forge:main Jun 21, 2024
12 checks passed
Copy link
Contributor

Hi! This is the friendly conda-forge automerge bot!

I considered the following status checks when analyzing this PR:

  • linter: passed
  • azure: passed

Thus the PR was passing and merged! Have a great day!

@srilman
Copy link
Contributor Author

srilman commented Jun 26, 2024

@h-vetinari Is there a way to rebuild the PyArrow conda-forge package to use the newest build of libarrow that contains this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Merge the PR when CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Conda Arrow Missing Azure FileSystem Support
3 participants