Skip to content

Conversation

@nateprewitt
Copy link

@nateprewitt nateprewitt commented Jan 23, 2026

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-actions
Copy link

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:

@nateprewitt nateprewitt changed the title Fix AzureFileSystem compilation on Windows GH-41990: [C++] Fix AzureFileSystem compilation on Windows Jan 23, 2026
@github-actions
Copy link

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

@h-vetinari
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants