diff --git a/.github/workflows/cpp.yml b/.github/workflows/cpp.yml index 287c0a581c28f..a4db36f52c6c2 100644 --- a/.github/workflows/cpp.yml +++ b/.github/workflows/cpp.yml @@ -228,7 +228,9 @@ jobs: run: ci/scripts/util_checkout.sh - name: Build shell: bash - run: ci/scripts/cpp_build.sh $(pwd) $(pwd)/build + run: | + export BOOST_ROOT=$BOOST_ROOT_1_72_0 + ci/scripts/cpp_build.sh $(pwd) $(pwd)/build - name: Test shell: bash run: ci/scripts/cpp_test.sh $(pwd) $(pwd)/build diff --git a/cpp/src/arrow/util/CMakeLists.txt b/cpp/src/arrow/util/CMakeLists.txt index 4c2ed4efa4fb5..ce08839d03e7c 100644 --- a/cpp/src/arrow/util/CMakeLists.txt +++ b/cpp/src/arrow/util/CMakeLists.txt @@ -26,6 +26,18 @@ arrow_install_all_headers("arrow/util") # arrow_test_main # +if(WIN32) + # This manifest enables long file paths on Windows 10+ + # See https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file#enable-long-paths-in-windows-10-version-1607-and-later + if(MSVC) + set(IO_UTIL_TEST_SOURCES io_util_test.cc io_util_test.manifest) + else() + set(IO_UTIL_TEST_SOURCES io_util_test.cc io_util_test.rc) + endif() +else() + set(IO_UTIL_TEST_SOURCES io_util_test.cc) +endif() + add_arrow_test(utility-test SOURCES align_util_test.cc @@ -37,7 +49,7 @@ add_arrow_test(utility-test key_value_metadata_test.cc hashing_test.cc int_util_test.cc - io_util_test.cc + ${IO_UTIL_TEST_SOURCES} iterator_test.cc logging_test.cc parsing_util_test.cc diff --git a/cpp/src/arrow/util/io_util.cc b/cpp/src/arrow/util/io_util.cc index c7efa15dd24ec..2e589fd2696e0 100644 --- a/cpp/src/arrow/util/io_util.cc +++ b/cpp/src/arrow/util/io_util.cc @@ -837,8 +837,19 @@ Result FileOpenReadable(const PlatformFilename& file_name) { int fd, errno_actual; #if defined(_WIN32) SetLastError(0); - errno_actual = _wsopen_s(&fd, file_name.ToNative().c_str(), - _O_RDONLY | _O_BINARY | _O_NOINHERIT, _SH_DENYNO, _S_IREAD); + HANDLE file_handle = CreateFileW(file_name.ToNative().c_str(), GENERIC_READ, + FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, + OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); + + DWORD last_error = GetLastError(); + if (last_error == ERROR_SUCCESS) { + errno_actual = 0; + fd = _open_osfhandle(reinterpret_cast(file_handle), + _O_RDONLY | _O_BINARY | _O_NOINHERIT); + } else { + return IOErrorFromWinError(last_error, "Failed to open local file '", + file_name.ToString(), "'"); + } #else fd = open(file_name.ToNative().c_str(), O_RDONLY); errno_actual = errno; @@ -868,23 +879,38 @@ Result FileOpenWritable(const PlatformFilename& file_name, bool write_only, #if defined(_WIN32) SetLastError(0); int oflag = _O_CREAT | _O_BINARY | _O_NOINHERIT; - int pmode = _S_IREAD | _S_IWRITE; + DWORD desired_access = GENERIC_WRITE; + DWORD share_mode = FILE_SHARE_READ | FILE_SHARE_WRITE; + DWORD creation_disposition = OPEN_ALWAYS; - if (truncate) { - oflag |= _O_TRUNC; - } if (append) { oflag |= _O_APPEND; } + if (truncate) { + oflag |= _O_TRUNC; + creation_disposition = CREATE_ALWAYS; + } + if (write_only) { oflag |= _O_WRONLY; } else { oflag |= _O_RDWR; + desired_access |= GENERIC_READ; } - errno_actual = _wsopen_s(&fd, file_name.ToNative().c_str(), oflag, _SH_DENYNO, pmode); + HANDLE file_handle = + CreateFileW(file_name.ToNative().c_str(), desired_access, share_mode, NULL, + creation_disposition, FILE_ATTRIBUTE_NORMAL, NULL); + DWORD last_error = GetLastError(); + if (last_error == ERROR_SUCCESS || last_error == ERROR_ALREADY_EXISTS) { + errno_actual = 0; + fd = _open_osfhandle(reinterpret_cast(file_handle), oflag); + } else { + return IOErrorFromWinError(last_error, "Failed to open local file '", + file_name.ToString(), "'"); + } #else int oflag = O_CREAT; diff --git a/cpp/src/arrow/util/io_util_test.cc b/cpp/src/arrow/util/io_util_test.cc index eb6ed0d9d92c4..b432d3367bdb5 100644 --- a/cpp/src/arrow/util/io_util_test.cc +++ b/cpp/src/arrow/util/io_util_test.cc @@ -394,6 +394,10 @@ TEST(DeleteDirContents, Basics) { #else ASSERT_EQ(ErrnoFromStatus(status), ENOENT); #endif + + // Now actually delete the test directory + ASSERT_OK_AND_ASSIGN(deleted, DeleteDirTree(parent)); + ASSERT_TRUE(deleted); } TEST(TemporaryDir, Basics) { @@ -523,5 +527,61 @@ TEST(DeleteFile, Basics) { ASSERT_RAISES(IOError, DeleteFile(fn)); } +#ifndef __APPLE__ +TEST(FileUtils, LongPaths) { + // ARROW-8477: check using long file paths under Windows (> 260 characters). + bool created, deleted; +#ifdef _WIN32 + const char* kRegKeyName = R"(SYSTEM\CurrentControlSet\Control\FileSystem)"; + const char* kRegValueName = "LongPathsEnabled"; + DWORD value = 0; + DWORD size = sizeof(value); + LSTATUS status = RegGetValueA(HKEY_LOCAL_MACHINE, kRegKeyName, kRegValueName, + RRF_RT_REG_DWORD, NULL, &value, &size); + bool test_long_paths = (status == ERROR_SUCCESS && value == 1); + if (!test_long_paths) { + ARROW_LOG(WARNING) + << "Tests for accessing files with long path names have been disabled. " + << "To enable these tests, set the value of " << kRegValueName + << " in registry key \\HKEY_LOCAL_MACHINE\\" << kRegKeyName + << " to 1 on the test host."; + return; + } +#endif + + const std::string BASE = "xxx-io-util-test-dir-long"; + PlatformFilename base_path, long_path, long_filename; + int fd = -1; + std::stringstream fs; + fs << BASE; + for (int i = 0; i < 64; ++i) { + fs << "/123456789ABCDEF"; + } + ASSERT_OK_AND_ASSIGN(base_path, + PlatformFilename::FromString(BASE)); // long_path length > 1024 + ASSERT_OK_AND_ASSIGN( + long_path, PlatformFilename::FromString(fs.str())); // long_path length > 1024 + ASSERT_OK_AND_ASSIGN(created, CreateDirTree(long_path)); + ASSERT_TRUE(created); + AssertExists(long_path); + ASSERT_OK_AND_ASSIGN(long_filename, + PlatformFilename::FromString(fs.str() + "/file.txt")); + ASSERT_OK_AND_ASSIGN(fd, FileOpenWritable(long_filename)); + ASSERT_OK(FileClose(fd)); + AssertExists(long_filename); + fd = -1; + ASSERT_OK_AND_ASSIGN(fd, FileOpenReadable(long_filename)); + ASSERT_OK(FileClose(fd)); + ASSERT_OK_AND_ASSIGN(deleted, DeleteDirContents(long_path)); + ASSERT_TRUE(deleted); + ASSERT_OK_AND_ASSIGN(deleted, DeleteDirTree(long_path)); + ASSERT_TRUE(deleted); + + // Now delete the whole test directory tree + ASSERT_OK_AND_ASSIGN(deleted, DeleteDirTree(base_path)); + ASSERT_TRUE(deleted); +} +#endif + } // namespace internal } // namespace arrow diff --git a/cpp/src/arrow/util/io_util_test.manifest b/cpp/src/arrow/util/io_util_test.manifest new file mode 100644 index 0000000000000..de5c0d8a875b1 --- /dev/null +++ b/cpp/src/arrow/util/io_util_test.manifest @@ -0,0 +1,39 @@ + + + + + + + + + true + + + + + + + + + + diff --git a/cpp/src/arrow/util/io_util_test.rc b/cpp/src/arrow/util/io_util_test.rc new file mode 100644 index 0000000000000..c3236cb2b3f88 --- /dev/null +++ b/cpp/src/arrow/util/io_util_test.rc @@ -0,0 +1,44 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#ifndef CREATEPROCESS_MANIFEST_RESOURCE_ID +#define CREATEPROCESS_MANIFEST_RESOURCE_ID 1 +#endif +#ifndef RT_MANIFEST +#define RT_MANIFEST 24 +#endif + +CREATEPROCESS_MANIFEST_RESOURCE_ID RT_MANIFEST +BEGIN + "" + "" + "" + "" + "" + "true" + "" + "" + "" + "" + "" + "" + "" + "" + "" + "" +END +