nateprewitt opened a new pull request, #48971:
URL: https://github.com/apache/arrow/pull/48971

   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](https://github.com/apache/arrow/issues/41990) 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 
8dd62d62a9af022813e9c8662956740340a9473f. 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.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to