GH-41990: [C++] Fix AzureFileSystem compilation on Windows

[nateprewitt]

Let me preface this pull request that I have not worked in C++ in quite a while. Apologies if this is missing modern idioms or is an obtuse fix.

Rationale for this change

I encountered an issue trying to compile the AzureFileSystem backend in C++ on Windows. Searching the issue tracker, it appears this is already a known but unresolved problem. This is an attempt to either address the issue or move the conversation forward for someone more experienced.

What changes are included in this PR?

AzureFileSystem uses unique_ptr while the other cloud file system implementations rely on shared_ptr. Since this is a forward-declared Impl in the headers file but the destructor was defined inline (via = default), we're getting compilation issues with MSVC due to it requiring the complete type earlier than GCC/Clang.

This change removes the defaulted definition from the header file and moves it into the .cc file where we have a complete type.

Unrelated, I've also wrapped 2 exception variables in ARROW_UNUSED. These are warnings treated as errors by MSVC at compile time. This was revealed in CI after resolving the issue above.

Are these changes tested?

I've enabled building and running the test suite in GHA in 8dd62d6. I believe a large portion of those tests may be skipped though since Azurite isn't present from what I can see. I'm not tied to the GHA updates being included in the PR, it's currently here for demonstration purposes. I noticed the other FS implementations are also not built and tested on Windows.

One quirk of this PR is getting WIL in place to compile the Azure C++ SDK was not intuitive for me. I've placed a dummy wilConfig.cmake to get the Azure SDK to build, but I'd assume there's a better way to do this. I'm happy to refine the build setup if we choose to keep it.

Are there any user-facing changes?

Nothing here should affect user-facing code beyond fixing the compilation issues. If there are concerns for things I'm missing, I'm happy to discuss those.

• GitHub Issue: [C++] Error when building Arrow-CPP w/ Azure Filesystem for Conda-Forge #41990

[github-actions]

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

See also:

• Other pull requests
• Contribution Guidelines - Contributing Overview

[github-actions]

⚠️ GitHub issue #41990 has been automatically assigned in GitHub to PR creator.

[h-vetinari]

Good job! I backported this to v23 in conda-forge/arrow-cpp-feedstock#1452 and it compiles! Unfortunately, the tests seem to hang... It's been stuck for >1h at

        Start  75: arrow-dataset-file-parquet-encryption-test
 75/104 Test  #75: arrow-dataset-file-parquet-encryption-test ...   Passed    1.90 sec
        Start  76: arrow-filesystem-test
 76/104 Test  #76: arrow-filesystem-test ........................   Passed   12.74 sec
        Start  77: arrow-azurefs-test

[kou]

Wow! I didn't notice the problem!

[nateprewitt]

I'm seeing a few failures on my forks CI. They appear to also be occurring with the tip of main and don't appear related. I'll leave them for now unless we're seeing other issues I'm missing.

[nateprewitt]

Good job! I backported this to v23 in conda-forge/arrow-cpp-feedstock#1452 and it compiles! Unfortunately, the tests seem to hang... It's been stuck for >1h at

        Start  75: arrow-dataset-file-parquet-encryption-test
 75/104 Test  #75: arrow-dataset-file-parquet-encryption-test ...   Passed    1.90 sec
        Start  76: arrow-filesystem-test
 76/104 Test  #76: arrow-filesystem-test ........................   Passed   12.74 sec
        Start  77: arrow-azurefs-test

@h-vetinari are you using the cpp_test.sh that's used in CI? A quick skim is showing these lines. If you do not have azurite installed in your testing environment, that test is being removed from running. You may want to use the test script or move some approximation of it into your test harness as it appears to be used in all GHA builds.

[h-vetinari]

In conda-forge we're using a very vanilla approach to testing the library:

:: cmake config etc.

cmake --build . --config Release
if %ERRORLEVEL% neq 0 exit 1

npm install -g azurite
set ARROW_TEST_DATA=%SRC_DIR%\testing\data
set PARQUET_TEST_DATA=%SRC_DIR%\cpp\submodules\parquet-testing\data
ctest --progress --output-on-failure
if %ERRORLEVEL% neq 0 exit 1

which has worked fine so far (and aside from batch->bash is also what we're using on unix for azureFS support already).

[nateprewitt]

Interesting, I was able to reproduce with your script. I'm going to post what I've found back in your original issue @h-vetinari so we can keep things isolated. The core of the issue is likely either with Azurite or the Azure C++ SDK with large payloads. It's not directly related to this PR but we should get it addressed as part of the larger fix.