Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion ci/conda_env_cpp.txt
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,7 +15,7 @@
# specific language governing permissions and limitations
# under the License.

aws-sdk-cpp
aws-sdk-cpp=1.8.186
benchmark>=1.6.0
boost-cpp>=1.68.0
brotli
Expand All@@ -25,13 +25,17 @@ cmake
gflags
glog
gmock>=1.10.0
google-cloud-cpp>=1.34.0
grpc-cpp>=1.27.3
gtest>=1.10.0
libprotobuf
libutf8proc
lz4-c
make
ninja
# Required by google-cloud-cpp, the Conda package is missing the dependency:
# https://github.com/conda-forge/google-cloud-cpp-feedstock/issues/28
nlohmann_json
pkg-config
python
rapidjson
Expand Down
9 changes: 6 additions & 3 deletions ci/docker/conda-cpp.dockerfile
Original file line numberDiff line numberDiff line change
Expand Up@@ -22,9 +22,6 @@ FROM ${repo}:${arch}-conda
COPY ci/scripts/install_minio.sh /arrow/ci/scripts
RUN /arrow/ci/scripts/install_minio.sh latest /opt/conda

COPY ci/scripts/install_gcs_testbench.sh /arrow/ci/scripts
RUN /arrow/ci/scripts/install_gcs_testbench.sh default

# install the required conda packages into the test environment
COPY ci/conda_env_cpp.txt \
ci/conda_env_gandiva.txt \
Expand All@@ -37,12 +34,17 @@ RUN mamba install \
valgrind && \
mamba clean --all

# We want to install the GCS testbench using the same Python binary that the Conda code will use.
COPY ci/scripts/install_gcs_testbench.sh /arrow/ci/scripts
RUN /arrow/ci/scripts/install_gcs_testbench.sh default

ENV ARROW_BUILD_TESTS=ON \
ARROW_DATASET=ON \
ARROW_DEPENDENCY_SOURCE=CONDA \
ARROW_FLIGHT=ON \
ARROW_FLIGHT_SQL=ON \
ARROW_GANDIVA=ON \
ARROW_GCS=ON \
ARROW_HOME=$CONDA_PREFIX \
ARROW_ORC=ON \
ARROW_PARQUET=ON \
Expand All@@ -55,6 +57,7 @@ ENV ARROW_BUILD_TESTS=ON \
ARROW_WITH_SNAPPY=ON \
ARROW_WITH_ZLIB=ON \
ARROW_WITH_ZSTD=ON \
CMAKE_CXX_STANDARD=17 \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Note this will break OSX for now, so it should only be set for Linux (as it is done here). See the tracking issue conda-forge/clang-compiler-activation-feedstock#17 for that. Can we remove the explicit setting of CMAKE_CXX_STANDARD in CMake instead?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Note this will break OSX for now, so it should only be set for Linux (as it is done here).

So this change is doing the "Right Thing"[tm] but maybe accidentally?

See the tracking issue conda-forge/clang-compiler-activation-feedstock#17 for that.

Ack.

Can we remove the explicit setting of CMAKE_CXX_STANDARD in CMake instead?

That is way above my pay grade (for this project). It depends on whether arrow supports any compiler where the default C++ version is < C++11. For example, GCC defaults to C++98 until GCC 6.x, and there is a similar story with Clang.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@pitrou do you have any thoughts on whether we can change the default for CMAKE_CXX_STANDARD?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm. I'm not sure I understand the situation entirely, but for now Arrow C++ needs to compile on gcc 4.9 (and perhaps even gcc 4.8, for a subset of Arrow). This is because of the compiler requirements for R packages...

@pitroupitrouDec 13, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also, unless I misunderstand @xhochy 's first message ("it should only be set for Linux (as it is done here)"), it seems there's no real problem here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

My message was mainly a heads-up that on OSX, you need to set CMAKE_CXX_STANDARD to 14. This is Linux here, so everything is fine here.

I'm not sure about the general removal of CMAKE_CXX_STANDARD as google-cloud-cpp does also set it: https://github.com/googleapis/google-cloud-cpp/blob/13ec1e946ae1baad6bcae952daf5910649dcfd0a/CMakeLists.txt#L31-L41 Possibly that combination could also be used here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Well, we already have the following:

if(NOTDEFINEDCMAKE_CXX_STANDARD)
set(CMAKE_CXX_STANDARD 11)
endif()

The only difference AFAICT is that we don't error out if the user explicitly asked for something earlier than C++11, but that must be a really rare case.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@xhochy Can you remind me the reason of CMAKE_CXX_STANDARD=17 here? Also, perhaps explain it in a comment?

GTest_SOURCE=BUNDLED \
PARQUET_BUILD_EXAMPLES=ON \
PARQUET_BUILD_EXECUTABLES=ON \
Expand Down
1 change: 1 addition & 0 deletions ci/scripts/cpp_build.sh
Original file line numberDiff line numberDiff line change
Expand Up@@ -135,6 +135,7 @@ cmake \
-DCMAKE_BUILD_TYPE=${ARROW_BUILD_TYPE:-debug} \
-DCMAKE_C_FLAGS="${CFLAGS:-}" \
-DCMAKE_CXX_FLAGS="${CXXFLAGS:-}" \
-DCMAKE_CXX_STANDARD="${CMAKE_CXX_STANDARD:-11}" \
-DCMAKE_INSTALL_LIBDIR=${CMAKE_INSTALL_LIBDIR:-lib} \
-DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX:-${ARROW_HOME}} \
-DCMAKE_UNITY_BUILD=${CMAKE_UNITY_BUILD:-OFF} \
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/filesystem/gcsfs.cc
Original file line numberDiff line numberDiff line change
Expand Up@@ -138,7 +138,7 @@ class GcsInputStream : public arrow::io::InputStream {
stream_.read(reinterpret_cast<char*>(buffer->mutable_data()), nbytes);
ARROW_GCS_RETURN_NOT_OK(stream_.status());
RETURN_NOT_OK(buffer->Resize(stream_.gcount(), true));
return buffer;
return std::shared_ptr<Buffer>(std::move(buffer));
}
//@}

Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
aws_sdk_cpp:
- 1.8.186
bzip2:
- '1'
c_compiler:
- gcc
c_compiler_version:
- '9'
- '7'
cdt_name:
- cos6
channel_sources:
- conda-forge,defaults
- conda-forge
channel_targets:
- conda-forge main
cuda_compiler:
Expand All@@ -19,23 +17,25 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '9'
- '7'
docker_image:
- quay.io/condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cos7-cuda:10.2
gflags:
- '2.2'
glog:
- '0.5'
grpc_cpp:
- '1.38'
- '1.42'
libprotobuf:
- '3.16'
- '3.19'
lz4_c:
- 1.9.3
numpy:
- '1.17'
- '1.18'
openssl:
- 1.1.1
orc:
- 1.6.8
- 1.7.2
pin_run_as_build:
bzip2:
max_pin: x
Expand All@@ -49,17 +49,17 @@ pin_run_as_build:
python:
- 3.7.* *_cpython
re2:
- 2021.06.01
- 2021.11.01
snappy:
- '1'
target_platform:
- linux-64
thrift_cpp:
- 0.14.2
- 0.15.0
zip_keys:
- - c_compiler_version
- cxx_compiler_version
- - cuda_compiler_version
- cuda_compiler_version
- cdt_name
- docker_image
- - python
Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
aws_sdk_cpp:
- 1.8.186
bzip2:
- '1'
c_compiler:
- gcc
c_compiler_version:
- '9'
- '7'
cdt_name:
- cos6
channel_sources:
- conda-forge,defaults
- conda-forge
channel_targets:
- conda-forge main
cuda_compiler:
Expand All@@ -19,23 +17,25 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '9'
- '7'
docker_image:
- quay.io/condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cos7-cuda:10.2
gflags:
- '2.2'
glog:
- '0.5'
grpc_cpp:
- '1.38'
- '1.42'
libprotobuf:
- '3.16'
- '3.19'
lz4_c:
- 1.9.3
numpy:
- '1.17'
- '1.18'
openssl:
- 1.1.1
orc:
- 1.6.8
- 1.7.2
pin_run_as_build:
bzip2:
max_pin: x
Expand All@@ -49,17 +49,17 @@ pin_run_as_build:
python:
- 3.8.* *_cpython
re2:
- 2021.06.01
- 2021.11.01
snappy:
- '1'
target_platform:
- linux-64
thrift_cpp:
- 0.14.2
- 0.15.0
zip_keys:
- - c_compiler_version
- cxx_compiler_version
- - cuda_compiler_version
- cuda_compiler_version
- cdt_name
- docker_image
- - python
Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
aws_sdk_cpp:
- 1.8.186
bzip2:
- '1'
c_compiler:
- gcc
c_compiler_version:
- '9'
- '7'
cdt_name:
- cos6
channel_sources:
- conda-forge,defaults
- conda-forge
channel_targets:
- conda-forge main
cuda_compiler:
Expand All@@ -19,23 +17,25 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '9'
- '7'
docker_image:
- quay.io/condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cos7-cuda:10.2
gflags:
- '2.2'
glog:
- '0.5'
grpc_cpp:
- '1.38'
- '1.42'
libprotobuf:
- '3.16'
- '3.19'
lz4_c:
- 1.9.3
numpy:
- '1.19'
openssl:
- 1.1.1
orc:
- 1.6.8
- 1.7.2
pin_run_as_build:
bzip2:
max_pin: x
Expand All@@ -49,17 +49,17 @@ pin_run_as_build:
python:
- 3.9.* *_cpython
re2:
- 2021.06.01
- 2021.11.01
snappy:
- '1'
target_platform:
- linux-64
thrift_cpp:
- 0.14.2
- 0.15.0
zip_keys:
- - c_compiler_version
- cxx_compiler_version
- - cuda_compiler_version
- cuda_compiler_version
- cdt_name
- docker_image
- - python
Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
aws_sdk_cpp:
- 1.8.186
bzip2:
- '1'
c_compiler:
- gcc
c_compiler_version:
- '9'
- '7'
cdt_name:
- cos6
channel_sources:
- conda-forge,defaults
- conda-forge
channel_targets:
- conda-forge main
cuda_compiler:
Expand All@@ -19,23 +17,25 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '9'
- '7'
docker_image:
- quay.io/condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cos7-cuda:10.2
gflags:
- '2.2'
glog:
- '0.5'
grpc_cpp:
- '1.38'
- '1.42'
libprotobuf:
- '3.16'
- '3.19'
lz4_c:
- 1.9.3
numpy:
- '1.17'
- '1.21'
openssl:
- 1.1.1
orc:
- 1.6.8
- 1.7.2
pin_run_as_build:
bzip2:
max_pin: x
Expand All@@ -47,19 +47,19 @@ pin_run_as_build:
zlib:
max_pin: x.x
python:
- 3.6.* *_cpython
- 3.10.* *_cpython
re2:
- 2021.06.01
- 2021.11.01
snappy:
- '1'
target_platform:
- linux-64
thrift_cpp:
- 0.14.2
- 0.15.0
zip_keys:
- - c_compiler_version
- cxx_compiler_version
- - cuda_compiler_version
- cuda_compiler_version
- cdt_name
- docker_image
- - python
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion ci/conda_env_cpp.txt
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,7 +15,7 @@
# specific language governing permissions and limitations
# under the License.

aws-sdk-cpp
aws-sdk-cpp=1.8.186
benchmark>=1.6.0
boost-cpp>=1.68.0
brotli
Expand All@@ -25,13 +25,17 @@ cmake
gflags
glog
gmock>=1.10.0
google-cloud-cpp>=1.34.0
grpc-cpp>=1.27.3
gtest>=1.10.0
libprotobuf
libutf8proc
lz4-c
make
ninja
# Required by google-cloud-cpp, the Conda package is missing the dependency:
# https://github.com/conda-forge/google-cloud-cpp-feedstock/issues/28
nlohmann_json
pkg-config
python
rapidjson
Expand Down
9 changes: 6 additions & 3 deletions ci/docker/conda-cpp.dockerfile
Original file line numberDiff line numberDiff line change
Expand Up@@ -22,9 +22,6 @@ FROM ${repo}:${arch}-conda
COPY ci/scripts/install_minio.sh /arrow/ci/scripts
RUN /arrow/ci/scripts/install_minio.sh latest /opt/conda

COPY ci/scripts/install_gcs_testbench.sh /arrow/ci/scripts
RUN /arrow/ci/scripts/install_gcs_testbench.sh default

# install the required conda packages into the test environment
COPY ci/conda_env_cpp.txt \
ci/conda_env_gandiva.txt \
Expand All@@ -37,12 +34,17 @@ RUN mamba install \
valgrind && \
mamba clean --all

# We want to install the GCS testbench using the same Python binary that the Conda code will use.
COPY ci/scripts/install_gcs_testbench.sh /arrow/ci/scripts
RUN /arrow/ci/scripts/install_gcs_testbench.sh default

ENV ARROW_BUILD_TESTS=ON \
ARROW_DATASET=ON \
ARROW_DEPENDENCY_SOURCE=CONDA \
ARROW_FLIGHT=ON \
ARROW_FLIGHT_SQL=ON \
ARROW_GANDIVA=ON \
ARROW_GCS=ON \
ARROW_HOME=$CONDA_PREFIX \
ARROW_ORC=ON \
ARROW_PARQUET=ON \
Expand All@@ -55,6 +57,7 @@ ENV ARROW_BUILD_TESTS=ON \
ARROW_WITH_SNAPPY=ON \
ARROW_WITH_ZLIB=ON \
ARROW_WITH_ZSTD=ON \
CMAKE_CXX_STANDARD=17 \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Note this will break OSX for now, so it should only be set for Linux (as it is done here). See the tracking issue conda-forge/clang-compiler-activation-feedstock#17 for that. Can we remove the explicit setting of CMAKE_CXX_STANDARD in CMake instead?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Note this will break OSX for now, so it should only be set for Linux (as it is done here).

So this change is doing the "Right Thing"[tm] but maybe accidentally?

See the tracking issue conda-forge/clang-compiler-activation-feedstock#17 for that.

Ack.

Can we remove the explicit setting of CMAKE_CXX_STANDARD in CMake instead?

That is way above my pay grade (for this project). It depends on whether arrow supports any compiler where the default C++ version is < C++11. For example, GCC defaults to C++98 until GCC 6.x, and there is a similar story with Clang.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@pitrou do you have any thoughts on whether we can change the default for CMAKE_CXX_STANDARD?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm. I'm not sure I understand the situation entirely, but for now Arrow C++ needs to compile on gcc 4.9 (and perhaps even gcc 4.8, for a subset of Arrow). This is because of the compiler requirements for R packages...

@pitroupitrouDec 13, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also, unless I misunderstand @xhochy 's first message ("it should only be set for Linux (as it is done here)"), it seems there's no real problem here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

My message was mainly a heads-up that on OSX, you need to set CMAKE_CXX_STANDARD to 14. This is Linux here, so everything is fine here.

I'm not sure about the general removal of CMAKE_CXX_STANDARD as google-cloud-cpp does also set it: https://github.com/googleapis/google-cloud-cpp/blob/13ec1e946ae1baad6bcae952daf5910649dcfd0a/CMakeLists.txt#L31-L41 Possibly that combination could also be used here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Well, we already have the following:

if(NOTDEFINEDCMAKE_CXX_STANDARD)
set(CMAKE_CXX_STANDARD 11)
endif()

The only difference AFAICT is that we don't error out if the user explicitly asked for something earlier than C++11, but that must be a really rare case.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@xhochy Can you remind me the reason of CMAKE_CXX_STANDARD=17 here? Also, perhaps explain it in a comment?

GTest_SOURCE=BUNDLED \
PARQUET_BUILD_EXAMPLES=ON \
PARQUET_BUILD_EXECUTABLES=ON \
Expand Down
1 change: 1 addition & 0 deletions ci/scripts/cpp_build.sh
Original file line numberDiff line numberDiff line change
Expand Up@@ -135,6 +135,7 @@ cmake \
-DCMAKE_BUILD_TYPE=${ARROW_BUILD_TYPE:-debug} \
-DCMAKE_C_FLAGS="${CFLAGS:-}" \
-DCMAKE_CXX_FLAGS="${CXXFLAGS:-}" \
-DCMAKE_CXX_STANDARD="${CMAKE_CXX_STANDARD:-11}" \
-DCMAKE_INSTALL_LIBDIR=${CMAKE_INSTALL_LIBDIR:-lib} \
-DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX:-${ARROW_HOME}} \
-DCMAKE_UNITY_BUILD=${CMAKE_UNITY_BUILD:-OFF} \
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/filesystem/gcsfs.cc
Original file line numberDiff line numberDiff line change
Expand Up@@ -138,7 +138,7 @@ class GcsInputStream : public arrow::io::InputStream {
stream_.read(reinterpret_cast<char*>(buffer->mutable_data()), nbytes);
ARROW_GCS_RETURN_NOT_OK(stream_.status());
RETURN_NOT_OK(buffer->Resize(stream_.gcount(), true));
return buffer;
return std::shared_ptr<Buffer>(std::move(buffer));
}
//@}

Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
aws_sdk_cpp:
- 1.8.186
bzip2:
- '1'
c_compiler:
- gcc
c_compiler_version:
- '9'
- '7'
cdt_name:
- cos6
channel_sources:
- conda-forge,defaults
- conda-forge
channel_targets:
- conda-forge main
cuda_compiler:
Expand All@@ -19,23 +17,25 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '9'
- '7'
docker_image:
- quay.io/condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cos7-cuda:10.2
gflags:
- '2.2'
glog:
- '0.5'
grpc_cpp:
- '1.38'
- '1.42'
libprotobuf:
- '3.16'
- '3.19'
lz4_c:
- 1.9.3
numpy:
- '1.17'
- '1.18'
openssl:
- 1.1.1
orc:
- 1.6.8
- 1.7.2
pin_run_as_build:
bzip2:
max_pin: x
Expand All@@ -49,17 +49,17 @@ pin_run_as_build:
python:
- 3.7.* *_cpython
re2:
- 2021.06.01
- 2021.11.01
snappy:
- '1'
target_platform:
- linux-64
thrift_cpp:
- 0.14.2
- 0.15.0
zip_keys:
- - c_compiler_version
- cxx_compiler_version
- - cuda_compiler_version
- cuda_compiler_version
- cdt_name
- docker_image
- - python
Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
aws_sdk_cpp:
- 1.8.186
bzip2:
- '1'
c_compiler:
- gcc
c_compiler_version:
- '9'
- '7'
cdt_name:
- cos6
channel_sources:
- conda-forge,defaults
- conda-forge
channel_targets:
- conda-forge main
cuda_compiler:
Expand All@@ -19,23 +17,25 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '9'
- '7'
docker_image:
- quay.io/condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cos7-cuda:10.2
gflags:
- '2.2'
glog:
- '0.5'
grpc_cpp:
- '1.38'
- '1.42'
libprotobuf:
- '3.16'
- '3.19'
lz4_c:
- 1.9.3
numpy:
- '1.17'
- '1.18'
openssl:
- 1.1.1
orc:
- 1.6.8
- 1.7.2
pin_run_as_build:
bzip2:
max_pin: x
Expand All@@ -49,17 +49,17 @@ pin_run_as_build:
python:
- 3.8.* *_cpython
re2:
- 2021.06.01
- 2021.11.01
snappy:
- '1'
target_platform:
- linux-64
thrift_cpp:
- 0.14.2
- 0.15.0
zip_keys:
- - c_compiler_version
- cxx_compiler_version
- - cuda_compiler_version
- cuda_compiler_version
- cdt_name
- docker_image
- - python
Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
aws_sdk_cpp:
- 1.8.186
bzip2:
- '1'
c_compiler:
- gcc
c_compiler_version:
- '9'
- '7'
cdt_name:
- cos6
channel_sources:
- conda-forge,defaults
- conda-forge
channel_targets:
- conda-forge main
cuda_compiler:
Expand All@@ -19,23 +17,25 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '9'
- '7'
docker_image:
- quay.io/condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cos7-cuda:10.2
gflags:
- '2.2'
glog:
- '0.5'
grpc_cpp:
- '1.38'
- '1.42'
libprotobuf:
- '3.16'
- '3.19'
lz4_c:
- 1.9.3
numpy:
- '1.19'
openssl:
- 1.1.1
orc:
- 1.6.8
- 1.7.2
pin_run_as_build:
bzip2:
max_pin: x
Expand All@@ -49,17 +49,17 @@ pin_run_as_build:
python:
- 3.9.* *_cpython
re2:
- 2021.06.01
- 2021.11.01
snappy:
- '1'
target_platform:
- linux-64
thrift_cpp:
- 0.14.2
- 0.15.0
zip_keys:
- - c_compiler_version
- cxx_compiler_version
- - cuda_compiler_version
- cuda_compiler_version
- cdt_name
- docker_image
- - python
Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
aws_sdk_cpp:
- 1.8.186
bzip2:
- '1'
c_compiler:
- gcc
c_compiler_version:
- '9'
- '7'
cdt_name:
- cos6
channel_sources:
- conda-forge,defaults
- conda-forge
channel_targets:
- conda-forge main
cuda_compiler:
Expand All@@ -19,23 +17,25 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '9'
- '7'
docker_image:
- quay.io/condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cos7-cuda:10.2
gflags:
- '2.2'
glog:
- '0.5'
grpc_cpp:
- '1.38'
- '1.42'
libprotobuf:
- '3.16'
- '3.19'
lz4_c:
- 1.9.3
numpy:
- '1.17'
- '1.21'
openssl:
- 1.1.1
orc:
- 1.6.8
- 1.7.2
pin_run_as_build:
bzip2:
max_pin: x
Expand All@@ -47,19 +47,19 @@ pin_run_as_build:
zlib:
max_pin: x.x
python:
- 3.6.* *_cpython
- 3.10.* *_cpython
re2:
- 2021.06.01
- 2021.11.01
snappy:
- '1'
target_platform:
- linux-64
thrift_cpp:
- 0.14.2
- 0.15.0
zip_keys:
- - c_compiler_version
- cxx_compiler_version
- - cuda_compiler_version
- cuda_compiler_version
- cdt_name
- docker_image
- - python
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion ci/conda_env_cpp.txt
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,7 +15,7 @@
# specific language governing permissions and limitations
# under the License.

aws-sdk-cpp
aws-sdk-cpp=1.8.186
benchmark>=1.6.0
boost-cpp>=1.68.0
brotli
Expand All@@ -25,13 +25,17 @@ cmake
gflags
glog
gmock>=1.10.0
google-cloud-cpp>=1.34.0
grpc-cpp>=1.27.3
gtest>=1.10.0
libprotobuf
libutf8proc
lz4-c
make
ninja
# Required by google-cloud-cpp, the Conda package is missing the dependency:
# https://github.com/conda-forge/google-cloud-cpp-feedstock/issues/28
nlohmann_json
pkg-config
python
rapidjson
Expand Down
9 changes: 6 additions & 3 deletions ci/docker/conda-cpp.dockerfile
Original file line numberDiff line numberDiff line change
Expand Up@@ -22,9 +22,6 @@ FROM ${repo}:${arch}-conda
COPY ci/scripts/install_minio.sh /arrow/ci/scripts
RUN /arrow/ci/scripts/install_minio.sh latest /opt/conda

COPY ci/scripts/install_gcs_testbench.sh /arrow/ci/scripts
RUN /arrow/ci/scripts/install_gcs_testbench.sh default

# install the required conda packages into the test environment
COPY ci/conda_env_cpp.txt \
ci/conda_env_gandiva.txt \
Expand All@@ -37,12 +34,17 @@ RUN mamba install \
valgrind && \
mamba clean --all

# We want to install the GCS testbench using the same Python binary that the Conda code will use.
COPY ci/scripts/install_gcs_testbench.sh /arrow/ci/scripts
RUN /arrow/ci/scripts/install_gcs_testbench.sh default

ENV ARROW_BUILD_TESTS=ON \
ARROW_DATASET=ON \
ARROW_DEPENDENCY_SOURCE=CONDA \
ARROW_FLIGHT=ON \
ARROW_FLIGHT_SQL=ON \
ARROW_GANDIVA=ON \
ARROW_GCS=ON \
ARROW_HOME=$CONDA_PREFIX \
ARROW_ORC=ON \
ARROW_PARQUET=ON \
Expand All@@ -55,6 +57,7 @@ ENV ARROW_BUILD_TESTS=ON \
ARROW_WITH_SNAPPY=ON \
ARROW_WITH_ZLIB=ON \
ARROW_WITH_ZSTD=ON \
CMAKE_CXX_STANDARD=17 \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Note this will break OSX for now, so it should only be set for Linux (as it is done here). See the tracking issue conda-forge/clang-compiler-activation-feedstock#17 for that. Can we remove the explicit setting of CMAKE_CXX_STANDARD in CMake instead?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Note this will break OSX for now, so it should only be set for Linux (as it is done here).

So this change is doing the "Right Thing"[tm] but maybe accidentally?

See the tracking issue conda-forge/clang-compiler-activation-feedstock#17 for that.

Ack.

Can we remove the explicit setting of CMAKE_CXX_STANDARD in CMake instead?

That is way above my pay grade (for this project). It depends on whether arrow supports any compiler where the default C++ version is < C++11. For example, GCC defaults to C++98 until GCC 6.x, and there is a similar story with Clang.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@pitrou do you have any thoughts on whether we can change the default for CMAKE_CXX_STANDARD?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm. I'm not sure I understand the situation entirely, but for now Arrow C++ needs to compile on gcc 4.9 (and perhaps even gcc 4.8, for a subset of Arrow). This is because of the compiler requirements for R packages...

@pitroupitrouDec 13, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also, unless I misunderstand @xhochy 's first message ("it should only be set for Linux (as it is done here)"), it seems there's no real problem here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

My message was mainly a heads-up that on OSX, you need to set CMAKE_CXX_STANDARD to 14. This is Linux here, so everything is fine here.

I'm not sure about the general removal of CMAKE_CXX_STANDARD as google-cloud-cpp does also set it: https://github.com/googleapis/google-cloud-cpp/blob/13ec1e946ae1baad6bcae952daf5910649dcfd0a/CMakeLists.txt#L31-L41 Possibly that combination could also be used here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Well, we already have the following:

if(NOTDEFINEDCMAKE_CXX_STANDARD)
set(CMAKE_CXX_STANDARD 11)
endif()

The only difference AFAICT is that we don't error out if the user explicitly asked for something earlier than C++11, but that must be a really rare case.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@xhochy Can you remind me the reason of CMAKE_CXX_STANDARD=17 here? Also, perhaps explain it in a comment?

GTest_SOURCE=BUNDLED \
PARQUET_BUILD_EXAMPLES=ON \
PARQUET_BUILD_EXECUTABLES=ON \
Expand Down
1 change: 1 addition & 0 deletions ci/scripts/cpp_build.sh
Original file line numberDiff line numberDiff line change
Expand Up@@ -135,6 +135,7 @@ cmake \
-DCMAKE_BUILD_TYPE=${ARROW_BUILD_TYPE:-debug} \
-DCMAKE_C_FLAGS="${CFLAGS:-}" \
-DCMAKE_CXX_FLAGS="${CXXFLAGS:-}" \
-DCMAKE_CXX_STANDARD="${CMAKE_CXX_STANDARD:-11}" \
-DCMAKE_INSTALL_LIBDIR=${CMAKE_INSTALL_LIBDIR:-lib} \
-DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX:-${ARROW_HOME}} \
-DCMAKE_UNITY_BUILD=${CMAKE_UNITY_BUILD:-OFF} \
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/filesystem/gcsfs.cc
Original file line numberDiff line numberDiff line change
Expand Up@@ -138,7 +138,7 @@ class GcsInputStream : public arrow::io::InputStream {
stream_.read(reinterpret_cast<char*>(buffer->mutable_data()), nbytes);
ARROW_GCS_RETURN_NOT_OK(stream_.status());
RETURN_NOT_OK(buffer->Resize(stream_.gcount(), true));
return buffer;
return std::shared_ptr<Buffer>(std::move(buffer));
}
//@}

Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
aws_sdk_cpp:
- 1.8.186
bzip2:
- '1'
c_compiler:
- gcc
c_compiler_version:
- '9'
- '7'
cdt_name:
- cos6
channel_sources:
- conda-forge,defaults
- conda-forge
channel_targets:
- conda-forge main
cuda_compiler:
Expand All@@ -19,23 +17,25 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '9'
- '7'
docker_image:
- quay.io/condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cos7-cuda:10.2
gflags:
- '2.2'
glog:
- '0.5'
grpc_cpp:
- '1.38'
- '1.42'
libprotobuf:
- '3.16'
- '3.19'
lz4_c:
- 1.9.3
numpy:
- '1.17'
- '1.18'
openssl:
- 1.1.1
orc:
- 1.6.8
- 1.7.2
pin_run_as_build:
bzip2:
max_pin: x
Expand All@@ -49,17 +49,17 @@ pin_run_as_build:
python:
- 3.7.* *_cpython
re2:
- 2021.06.01
- 2021.11.01
snappy:
- '1'
target_platform:
- linux-64
thrift_cpp:
- 0.14.2
- 0.15.0
zip_keys:
- - c_compiler_version
- cxx_compiler_version
- - cuda_compiler_version
- cuda_compiler_version
- cdt_name
- docker_image
- - python
Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
aws_sdk_cpp:
- 1.8.186
bzip2:
- '1'
c_compiler:
- gcc
c_compiler_version:
- '9'
- '7'
cdt_name:
- cos6
channel_sources:
- conda-forge,defaults
- conda-forge
channel_targets:
- conda-forge main
cuda_compiler:
Expand All@@ -19,23 +17,25 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '9'
- '7'
docker_image:
- quay.io/condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cos7-cuda:10.2
gflags:
- '2.2'
glog:
- '0.5'
grpc_cpp:
- '1.38'
- '1.42'
libprotobuf:
- '3.16'
- '3.19'
lz4_c:
- 1.9.3
numpy:
- '1.17'
- '1.18'
openssl:
- 1.1.1
orc:
- 1.6.8
- 1.7.2
pin_run_as_build:
bzip2:
max_pin: x
Expand All@@ -49,17 +49,17 @@ pin_run_as_build:
python:
- 3.8.* *_cpython
re2:
- 2021.06.01
- 2021.11.01
snappy:
- '1'
target_platform:
- linux-64
thrift_cpp:
- 0.14.2
- 0.15.0
zip_keys:
- - c_compiler_version
- cxx_compiler_version
- - cuda_compiler_version
- cuda_compiler_version
- cdt_name
- docker_image
- - python
Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
aws_sdk_cpp:
- 1.8.186
bzip2:
- '1'
c_compiler:
- gcc
c_compiler_version:
- '9'
- '7'
cdt_name:
- cos6
channel_sources:
- conda-forge,defaults
- conda-forge
channel_targets:
- conda-forge main
cuda_compiler:
Expand All@@ -19,23 +17,25 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '9'
- '7'
docker_image:
- quay.io/condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cos7-cuda:10.2
gflags:
- '2.2'
glog:
- '0.5'
grpc_cpp:
- '1.38'
- '1.42'
libprotobuf:
- '3.16'
- '3.19'
lz4_c:
- 1.9.3
numpy:
- '1.19'
openssl:
- 1.1.1
orc:
- 1.6.8
- 1.7.2
pin_run_as_build:
bzip2:
max_pin: x
Expand All@@ -49,17 +49,17 @@ pin_run_as_build:
python:
- 3.9.* *_cpython
re2:
- 2021.06.01
- 2021.11.01
snappy:
- '1'
target_platform:
- linux-64
thrift_cpp:
- 0.14.2
- 0.15.0
zip_keys:
- - c_compiler_version
- cxx_compiler_version
- - cuda_compiler_version
- cuda_compiler_version
- cdt_name
- docker_image
- - python
Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
aws_sdk_cpp:
- 1.8.186
bzip2:
- '1'
c_compiler:
- gcc
c_compiler_version:
- '9'
- '7'
cdt_name:
- cos6
channel_sources:
- conda-forge,defaults
- conda-forge
channel_targets:
- conda-forge main
cuda_compiler:
Expand All@@ -19,23 +17,25 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '9'
- '7'
docker_image:
- quay.io/condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cos7-cuda:10.2
gflags:
- '2.2'
glog:
- '0.5'
grpc_cpp:
- '1.38'
- '1.42'
libprotobuf:
- '3.16'
- '3.19'
lz4_c:
- 1.9.3
numpy:
- '1.17'
- '1.21'
openssl:
- 1.1.1
orc:
- 1.6.8
- 1.7.2
pin_run_as_build:
bzip2:
max_pin: x
Expand All@@ -47,19 +47,19 @@ pin_run_as_build:
zlib:
max_pin: x.x
python:
- 3.6.* *_cpython
- 3.10.* *_cpython
re2:
- 2021.06.01
- 2021.11.01
snappy:
- '1'
target_platform:
- linux-64
thrift_cpp:
- 0.14.2
- 0.15.0
zip_keys:
- - c_compiler_version
- cxx_compiler_version
- - cuda_compiler_version
- cuda_compiler_version
- cdt_name
- docker_image
- - python
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion ci/conda_env_cpp.txt
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,7 +15,7 @@
# specific language governing permissions and limitations
# under the License.

aws-sdk-cpp
aws-sdk-cpp=1.8.186
benchmark>=1.6.0
boost-cpp>=1.68.0
brotli
Expand All@@ -25,13 +25,17 @@ cmake
gflags
glog
gmock>=1.10.0
google-cloud-cpp>=1.34.0
grpc-cpp>=1.27.3
gtest>=1.10.0
libprotobuf
libutf8proc
lz4-c
make
ninja
# Required by google-cloud-cpp, the Conda package is missing the dependency:
# https://github.com/conda-forge/google-cloud-cpp-feedstock/issues/28
nlohmann_json
pkg-config
python
rapidjson
Expand Down
9 changes: 6 additions & 3 deletions ci/docker/conda-cpp.dockerfile
Original file line numberDiff line numberDiff line change
Expand Up@@ -22,9 +22,6 @@ FROM ${repo}:${arch}-conda
COPY ci/scripts/install_minio.sh /arrow/ci/scripts
RUN /arrow/ci/scripts/install_minio.sh latest /opt/conda

COPY ci/scripts/install_gcs_testbench.sh /arrow/ci/scripts
RUN /arrow/ci/scripts/install_gcs_testbench.sh default

# install the required conda packages into the test environment
COPY ci/conda_env_cpp.txt \
ci/conda_env_gandiva.txt \
Expand All@@ -37,12 +34,17 @@ RUN mamba install \
valgrind && \
mamba clean --all

# We want to install the GCS testbench using the same Python binary that the Conda code will use.
COPY ci/scripts/install_gcs_testbench.sh /arrow/ci/scripts
RUN /arrow/ci/scripts/install_gcs_testbench.sh default

ENV ARROW_BUILD_TESTS=ON \
ARROW_DATASET=ON \
ARROW_DEPENDENCY_SOURCE=CONDA \
ARROW_FLIGHT=ON \
ARROW_FLIGHT_SQL=ON \
ARROW_GANDIVA=ON \
ARROW_GCS=ON \
ARROW_HOME=$CONDA_PREFIX \
ARROW_ORC=ON \
ARROW_PARQUET=ON \
Expand All@@ -55,6 +57,7 @@ ENV ARROW_BUILD_TESTS=ON \
ARROW_WITH_SNAPPY=ON \
ARROW_WITH_ZLIB=ON \
ARROW_WITH_ZSTD=ON \
CMAKE_CXX_STANDARD=17 \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Note this will break OSX for now, so it should only be set for Linux (as it is done here). See the tracking issue conda-forge/clang-compiler-activation-feedstock#17 for that. Can we remove the explicit setting of CMAKE_CXX_STANDARD in CMake instead?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Note this will break OSX for now, so it should only be set for Linux (as it is done here).

So this change is doing the "Right Thing"[tm] but maybe accidentally?

See the tracking issue conda-forge/clang-compiler-activation-feedstock#17 for that.

Ack.

Can we remove the explicit setting of CMAKE_CXX_STANDARD in CMake instead?

That is way above my pay grade (for this project). It depends on whether arrow supports any compiler where the default C++ version is < C++11. For example, GCC defaults to C++98 until GCC 6.x, and there is a similar story with Clang.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@pitrou do you have any thoughts on whether we can change the default for CMAKE_CXX_STANDARD?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm. I'm not sure I understand the situation entirely, but for now Arrow C++ needs to compile on gcc 4.9 (and perhaps even gcc 4.8, for a subset of Arrow). This is because of the compiler requirements for R packages...

@pitroupitrouDec 13, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also, unless I misunderstand @xhochy 's first message ("it should only be set for Linux (as it is done here)"), it seems there's no real problem here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

My message was mainly a heads-up that on OSX, you need to set CMAKE_CXX_STANDARD to 14. This is Linux here, so everything is fine here.

I'm not sure about the general removal of CMAKE_CXX_STANDARD as google-cloud-cpp does also set it: https://github.com/googleapis/google-cloud-cpp/blob/13ec1e946ae1baad6bcae952daf5910649dcfd0a/CMakeLists.txt#L31-L41 Possibly that combination could also be used here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Well, we already have the following:

if(NOTDEFINEDCMAKE_CXX_STANDARD)
set(CMAKE_CXX_STANDARD 11)
endif()

The only difference AFAICT is that we don't error out if the user explicitly asked for something earlier than C++11, but that must be a really rare case.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@xhochy Can you remind me the reason of CMAKE_CXX_STANDARD=17 here? Also, perhaps explain it in a comment?

GTest_SOURCE=BUNDLED \
PARQUET_BUILD_EXAMPLES=ON \
PARQUET_BUILD_EXECUTABLES=ON \
Expand Down
1 change: 1 addition & 0 deletions ci/scripts/cpp_build.sh
Original file line numberDiff line numberDiff line change
Expand Up@@ -135,6 +135,7 @@ cmake \
-DCMAKE_BUILD_TYPE=${ARROW_BUILD_TYPE:-debug} \
-DCMAKE_C_FLAGS="${CFLAGS:-}" \
-DCMAKE_CXX_FLAGS="${CXXFLAGS:-}" \
-DCMAKE_CXX_STANDARD="${CMAKE_CXX_STANDARD:-11}" \
-DCMAKE_INSTALL_LIBDIR=${CMAKE_INSTALL_LIBDIR:-lib} \
-DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX:-${ARROW_HOME}} \
-DCMAKE_UNITY_BUILD=${CMAKE_UNITY_BUILD:-OFF} \
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/filesystem/gcsfs.cc
Original file line numberDiff line numberDiff line change
Expand Up@@ -138,7 +138,7 @@ class GcsInputStream : public arrow::io::InputStream {
stream_.read(reinterpret_cast<char*>(buffer->mutable_data()), nbytes);
ARROW_GCS_RETURN_NOT_OK(stream_.status());
RETURN_NOT_OK(buffer->Resize(stream_.gcount(), true));
return buffer;
return std::shared_ptr<Buffer>(std::move(buffer));
}
//@}

Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
aws_sdk_cpp:
- 1.8.186
bzip2:
- '1'
c_compiler:
- gcc
c_compiler_version:
- '9'
- '7'
cdt_name:
- cos6
channel_sources:
- conda-forge,defaults
- conda-forge
channel_targets:
- conda-forge main
cuda_compiler:
Expand All@@ -19,23 +17,25 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '9'
- '7'
docker_image:
- quay.io/condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cos7-cuda:10.2
gflags:
- '2.2'
glog:
- '0.5'
grpc_cpp:
- '1.38'
- '1.42'
libprotobuf:
- '3.16'
- '3.19'
lz4_c:
- 1.9.3
numpy:
- '1.17'
- '1.18'
openssl:
- 1.1.1
orc:
- 1.6.8
- 1.7.2
pin_run_as_build:
bzip2:
max_pin: x
Expand All@@ -49,17 +49,17 @@ pin_run_as_build:
python:
- 3.7.* *_cpython
re2:
- 2021.06.01
- 2021.11.01
snappy:
- '1'
target_platform:
- linux-64
thrift_cpp:
- 0.14.2
- 0.15.0
zip_keys:
- - c_compiler_version
- cxx_compiler_version
- - cuda_compiler_version
- cuda_compiler_version
- cdt_name
- docker_image
- - python
Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
aws_sdk_cpp:
- 1.8.186
bzip2:
- '1'
c_compiler:
- gcc
c_compiler_version:
- '9'
- '7'
cdt_name:
- cos6
channel_sources:
- conda-forge,defaults
- conda-forge
channel_targets:
- conda-forge main
cuda_compiler:
Expand All@@ -19,23 +17,25 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '9'
- '7'
docker_image:
- quay.io/condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cos7-cuda:10.2
gflags:
- '2.2'
glog:
- '0.5'
grpc_cpp:
- '1.38'
- '1.42'
libprotobuf:
- '3.16'
- '3.19'
lz4_c:
- 1.9.3
numpy:
- '1.17'
- '1.18'
openssl:
- 1.1.1
orc:
- 1.6.8
- 1.7.2
pin_run_as_build:
bzip2:
max_pin: x
Expand All@@ -49,17 +49,17 @@ pin_run_as_build:
python:
- 3.8.* *_cpython
re2:
- 2021.06.01
- 2021.11.01
snappy:
- '1'
target_platform:
- linux-64
thrift_cpp:
- 0.14.2
- 0.15.0
zip_keys:
- - c_compiler_version
- cxx_compiler_version
- - cuda_compiler_version
- cuda_compiler_version
- cdt_name
- docker_image
- - python
Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
aws_sdk_cpp:
- 1.8.186
bzip2:
- '1'
c_compiler:
- gcc
c_compiler_version:
- '9'
- '7'
cdt_name:
- cos6
channel_sources:
- conda-forge,defaults
- conda-forge
channel_targets:
- conda-forge main
cuda_compiler:
Expand All@@ -19,23 +17,25 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '9'
- '7'
docker_image:
- quay.io/condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cos7-cuda:10.2
gflags:
- '2.2'
glog:
- '0.5'
grpc_cpp:
- '1.38'
- '1.42'
libprotobuf:
- '3.16'
- '3.19'
lz4_c:
- 1.9.3
numpy:
- '1.19'
openssl:
- 1.1.1
orc:
- 1.6.8
- 1.7.2
pin_run_as_build:
bzip2:
max_pin: x
Expand All@@ -49,17 +49,17 @@ pin_run_as_build:
python:
- 3.9.* *_cpython
re2:
- 2021.06.01
- 2021.11.01
snappy:
- '1'
target_platform:
- linux-64
thrift_cpp:
- 0.14.2
- 0.15.0
zip_keys:
- - c_compiler_version
- cxx_compiler_version
- - cuda_compiler_version
- cuda_compiler_version
- cdt_name
- docker_image
- - python
Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
aws_sdk_cpp:
- 1.8.186
bzip2:
- '1'
c_compiler:
- gcc
c_compiler_version:
- '9'
- '7'
cdt_name:
- cos6
channel_sources:
- conda-forge,defaults
- conda-forge
channel_targets:
- conda-forge main
cuda_compiler:
Expand All@@ -19,23 +17,25 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '9'
- '7'
docker_image:
- quay.io/condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cos7-cuda:10.2
gflags:
- '2.2'
glog:
- '0.5'
grpc_cpp:
- '1.38'
- '1.42'
libprotobuf:
- '3.16'
- '3.19'
lz4_c:
- 1.9.3
numpy:
- '1.17'
- '1.21'
openssl:
- 1.1.1
orc:
- 1.6.8
- 1.7.2
pin_run_as_build:
bzip2:
max_pin: x
Expand All@@ -47,19 +47,19 @@ pin_run_as_build:
zlib:
max_pin: x.x
python:
- 3.6.* *_cpython
- 3.10.* *_cpython
re2:
- 2021.06.01
- 2021.11.01
snappy:
- '1'
target_platform:
- linux-64
thrift_cpp:
- 0.14.2
- 0.15.0
zip_keys:
- - c_compiler_version
- cxx_compiler_version
- - cuda_compiler_version
- cuda_compiler_version
- cdt_name
- docker_image
- - python
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion ci/conda_env_cpp.txt
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,7 +15,7 @@
# specific language governing permissions and limitations
# under the License.

aws-sdk-cpp
aws-sdk-cpp=1.8.186
benchmark>=1.6.0
boost-cpp>=1.68.0
brotli
Expand All@@ -25,13 +25,17 @@ cmake
gflags
glog
gmock>=1.10.0
google-cloud-cpp>=1.34.0
grpc-cpp>=1.27.3
gtest>=1.10.0
libprotobuf
libutf8proc
lz4-c
make
ninja
# Required by google-cloud-cpp, the Conda package is missing the dependency:
# https://github.com/conda-forge/google-cloud-cpp-feedstock/issues/28
nlohmann_json
pkg-config
python
rapidjson
Expand Down
9 changes: 6 additions & 3 deletions ci/docker/conda-cpp.dockerfile
Original file line numberDiff line numberDiff line change
Expand Up@@ -22,9 +22,6 @@ FROM ${repo}:${arch}-conda
COPY ci/scripts/install_minio.sh /arrow/ci/scripts
RUN /arrow/ci/scripts/install_minio.sh latest /opt/conda

COPY ci/scripts/install_gcs_testbench.sh /arrow/ci/scripts
RUN /arrow/ci/scripts/install_gcs_testbench.sh default

# install the required conda packages into the test environment
COPY ci/conda_env_cpp.txt \
ci/conda_env_gandiva.txt \
Expand All@@ -37,12 +34,17 @@ RUN mamba install \
valgrind && \
mamba clean --all

# We want to install the GCS testbench using the same Python binary that the Conda code will use.
COPY ci/scripts/install_gcs_testbench.sh /arrow/ci/scripts
RUN /arrow/ci/scripts/install_gcs_testbench.sh default

ENV ARROW_BUILD_TESTS=ON \
ARROW_DATASET=ON \
ARROW_DEPENDENCY_SOURCE=CONDA \
ARROW_FLIGHT=ON \
ARROW_FLIGHT_SQL=ON \
ARROW_GANDIVA=ON \
ARROW_GCS=ON \
ARROW_HOME=$CONDA_PREFIX \
ARROW_ORC=ON \
ARROW_PARQUET=ON \
Expand All@@ -55,6 +57,7 @@ ENV ARROW_BUILD_TESTS=ON \
ARROW_WITH_SNAPPY=ON \
ARROW_WITH_ZLIB=ON \
ARROW_WITH_ZSTD=ON \
CMAKE_CXX_STANDARD=17 \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Note this will break OSX for now, so it should only be set for Linux (as it is done here). See the tracking issue conda-forge/clang-compiler-activation-feedstock#17 for that. Can we remove the explicit setting of CMAKE_CXX_STANDARD in CMake instead?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Note this will break OSX for now, so it should only be set for Linux (as it is done here).

So this change is doing the "Right Thing"[tm] but maybe accidentally?

See the tracking issue conda-forge/clang-compiler-activation-feedstock#17 for that.

Ack.

Can we remove the explicit setting of CMAKE_CXX_STANDARD in CMake instead?

That is way above my pay grade (for this project). It depends on whether arrow supports any compiler where the default C++ version is < C++11. For example, GCC defaults to C++98 until GCC 6.x, and there is a similar story with Clang.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@pitrou do you have any thoughts on whether we can change the default for CMAKE_CXX_STANDARD?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm. I'm not sure I understand the situation entirely, but for now Arrow C++ needs to compile on gcc 4.9 (and perhaps even gcc 4.8, for a subset of Arrow). This is because of the compiler requirements for R packages...

@pitroupitrouDec 13, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also, unless I misunderstand @xhochy 's first message ("it should only be set for Linux (as it is done here)"), it seems there's no real problem here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

My message was mainly a heads-up that on OSX, you need to set CMAKE_CXX_STANDARD to 14. This is Linux here, so everything is fine here.

I'm not sure about the general removal of CMAKE_CXX_STANDARD as google-cloud-cpp does also set it: https://github.com/googleapis/google-cloud-cpp/blob/13ec1e946ae1baad6bcae952daf5910649dcfd0a/CMakeLists.txt#L31-L41 Possibly that combination could also be used here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Well, we already have the following:

if(NOTDEFINEDCMAKE_CXX_STANDARD)
set(CMAKE_CXX_STANDARD 11)
endif()

The only difference AFAICT is that we don't error out if the user explicitly asked for something earlier than C++11, but that must be a really rare case.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@xhochy Can you remind me the reason of CMAKE_CXX_STANDARD=17 here? Also, perhaps explain it in a comment?

GTest_SOURCE=BUNDLED \
PARQUET_BUILD_EXAMPLES=ON \
PARQUET_BUILD_EXECUTABLES=ON \
Expand Down
1 change: 1 addition & 0 deletions ci/scripts/cpp_build.sh
Original file line numberDiff line numberDiff line change
Expand Up@@ -135,6 +135,7 @@ cmake \
-DCMAKE_BUILD_TYPE=${ARROW_BUILD_TYPE:-debug} \
-DCMAKE_C_FLAGS="${CFLAGS:-}" \
-DCMAKE_CXX_FLAGS="${CXXFLAGS:-}" \
-DCMAKE_CXX_STANDARD="${CMAKE_CXX_STANDARD:-11}" \
-DCMAKE_INSTALL_LIBDIR=${CMAKE_INSTALL_LIBDIR:-lib} \
-DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX:-${ARROW_HOME}} \
-DCMAKE_UNITY_BUILD=${CMAKE_UNITY_BUILD:-OFF} \
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/filesystem/gcsfs.cc
Original file line numberDiff line numberDiff line change
Expand Up@@ -138,7 +138,7 @@ class GcsInputStream : public arrow::io::InputStream {
stream_.read(reinterpret_cast<char*>(buffer->mutable_data()), nbytes);
ARROW_GCS_RETURN_NOT_OK(stream_.status());
RETURN_NOT_OK(buffer->Resize(stream_.gcount(), true));
return buffer;
return std::shared_ptr<Buffer>(std::move(buffer));
}
//@}

Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
aws_sdk_cpp:
- 1.8.186
bzip2:
- '1'
c_compiler:
- gcc
c_compiler_version:
- '9'
- '7'
cdt_name:
- cos6
channel_sources:
- conda-forge,defaults
- conda-forge
channel_targets:
- conda-forge main
cuda_compiler:
Expand All@@ -19,23 +17,25 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '9'
- '7'
docker_image:
- quay.io/condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cos7-cuda:10.2
gflags:
- '2.2'
glog:
- '0.5'
grpc_cpp:
- '1.38'
- '1.42'
libprotobuf:
- '3.16'
- '3.19'
lz4_c:
- 1.9.3
numpy:
- '1.17'
- '1.18'
openssl:
- 1.1.1
orc:
- 1.6.8
- 1.7.2
pin_run_as_build:
bzip2:
max_pin: x
Expand All@@ -49,17 +49,17 @@ pin_run_as_build:
python:
- 3.7.* *_cpython
re2:
- 2021.06.01
- 2021.11.01
snappy:
- '1'
target_platform:
- linux-64
thrift_cpp:
- 0.14.2
- 0.15.0
zip_keys:
- - c_compiler_version
- cxx_compiler_version
- - cuda_compiler_version
- cuda_compiler_version
- cdt_name
- docker_image
- - python
Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
aws_sdk_cpp:
- 1.8.186
bzip2:
- '1'
c_compiler:
- gcc
c_compiler_version:
- '9'
- '7'
cdt_name:
- cos6
channel_sources:
- conda-forge,defaults
- conda-forge
channel_targets:
- conda-forge main
cuda_compiler:
Expand All@@ -19,23 +17,25 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '9'
- '7'
docker_image:
- quay.io/condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cos7-cuda:10.2
gflags:
- '2.2'
glog:
- '0.5'
grpc_cpp:
- '1.38'
- '1.42'
libprotobuf:
- '3.16'
- '3.19'
lz4_c:
- 1.9.3
numpy:
- '1.17'
- '1.18'
openssl:
- 1.1.1
orc:
- 1.6.8
- 1.7.2
pin_run_as_build:
bzip2:
max_pin: x
Expand All@@ -49,17 +49,17 @@ pin_run_as_build:
python:
- 3.8.* *_cpython
re2:
- 2021.06.01
- 2021.11.01
snappy:
- '1'
target_platform:
- linux-64
thrift_cpp:
- 0.14.2
- 0.15.0
zip_keys:
- - c_compiler_version
- cxx_compiler_version
- - cuda_compiler_version
- cuda_compiler_version
- cdt_name
- docker_image
- - python
Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
aws_sdk_cpp:
- 1.8.186
bzip2:
- '1'
c_compiler:
- gcc
c_compiler_version:
- '9'
- '7'
cdt_name:
- cos6
channel_sources:
- conda-forge,defaults
- conda-forge
channel_targets:
- conda-forge main
cuda_compiler:
Expand All@@ -19,23 +17,25 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '9'
- '7'
docker_image:
- quay.io/condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cos7-cuda:10.2
gflags:
- '2.2'
glog:
- '0.5'
grpc_cpp:
- '1.38'
- '1.42'
libprotobuf:
- '3.16'
- '3.19'
lz4_c:
- 1.9.3
numpy:
- '1.19'
openssl:
- 1.1.1
orc:
- 1.6.8
- 1.7.2
pin_run_as_build:
bzip2:
max_pin: x
Expand All@@ -49,17 +49,17 @@ pin_run_as_build:
python:
- 3.9.* *_cpython
re2:
- 2021.06.01
- 2021.11.01
snappy:
- '1'
target_platform:
- linux-64
thrift_cpp:
- 0.14.2
- 0.15.0
zip_keys:
- - c_compiler_version
- cxx_compiler_version
- - cuda_compiler_version
- cuda_compiler_version
- cdt_name
- docker_image
- - python
Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
aws_sdk_cpp:
- 1.8.186
bzip2:
- '1'
c_compiler:
- gcc
c_compiler_version:
- '9'
- '7'
cdt_name:
- cos6
channel_sources:
- conda-forge,defaults
- conda-forge
channel_targets:
- conda-forge main
cuda_compiler:
Expand All@@ -19,23 +17,25 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '9'
- '7'
docker_image:
- quay.io/condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cos7-cuda:10.2
gflags:
- '2.2'
glog:
- '0.5'
grpc_cpp:
- '1.38'
- '1.42'
libprotobuf:
- '3.16'
- '3.19'
lz4_c:
- 1.9.3
numpy:
- '1.17'
- '1.21'
openssl:
- 1.1.1
orc:
- 1.6.8
- 1.7.2
pin_run_as_build:
bzip2:
max_pin: x
Expand All@@ -47,19 +47,19 @@ pin_run_as_build:
zlib:
max_pin: x.x
python:
- 3.6.* *_cpython
- 3.10.* *_cpython
re2:
- 2021.06.01
- 2021.11.01
snappy:
- '1'
target_platform:
- linux-64
thrift_cpp:
- 0.14.2
- 0.15.0
zip_keys:
- - c_compiler_version
- cxx_compiler_version
- - cuda_compiler_version
- cuda_compiler_version
- cdt_name
- docker_image
- - python
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion ci/conda_env_cpp.txt
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,7 +15,7 @@
# specific language governing permissions and limitations
# under the License.

aws-sdk-cpp
aws-sdk-cpp=1.8.186
benchmark>=1.6.0
boost-cpp>=1.68.0
brotli
Expand All@@ -25,13 +25,17 @@ cmake
gflags
glog
gmock>=1.10.0
google-cloud-cpp>=1.34.0
grpc-cpp>=1.27.3
gtest>=1.10.0
libprotobuf
libutf8proc
lz4-c
make
ninja
# Required by google-cloud-cpp, the Conda package is missing the dependency:
# https://github.com/conda-forge/google-cloud-cpp-feedstock/issues/28
nlohmann_json
pkg-config
python
rapidjson
Expand Down
9 changes: 6 additions & 3 deletions ci/docker/conda-cpp.dockerfile
Original file line numberDiff line numberDiff line change
Expand Up@@ -22,9 +22,6 @@ FROM ${repo}:${arch}-conda
COPY ci/scripts/install_minio.sh /arrow/ci/scripts
RUN /arrow/ci/scripts/install_minio.sh latest /opt/conda

COPY ci/scripts/install_gcs_testbench.sh /arrow/ci/scripts
RUN /arrow/ci/scripts/install_gcs_testbench.sh default

# install the required conda packages into the test environment
COPY ci/conda_env_cpp.txt \
ci/conda_env_gandiva.txt \
Expand All@@ -37,12 +34,17 @@ RUN mamba install \
valgrind && \
mamba clean --all

# We want to install the GCS testbench using the same Python binary that the Conda code will use.
COPY ci/scripts/install_gcs_testbench.sh /arrow/ci/scripts
RUN /arrow/ci/scripts/install_gcs_testbench.sh default

ENV ARROW_BUILD_TESTS=ON \
ARROW_DATASET=ON \
ARROW_DEPENDENCY_SOURCE=CONDA \
ARROW_FLIGHT=ON \
ARROW_FLIGHT_SQL=ON \
ARROW_GANDIVA=ON \
ARROW_GCS=ON \
ARROW_HOME=$CONDA_PREFIX \
ARROW_ORC=ON \
ARROW_PARQUET=ON \
Expand All@@ -55,6 +57,7 @@ ENV ARROW_BUILD_TESTS=ON \
ARROW_WITH_SNAPPY=ON \
ARROW_WITH_ZLIB=ON \
ARROW_WITH_ZSTD=ON \
CMAKE_CXX_STANDARD=17 \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Note this will break OSX for now, so it should only be set for Linux (as it is done here). See the tracking issue conda-forge/clang-compiler-activation-feedstock#17 for that. Can we remove the explicit setting of CMAKE_CXX_STANDARD in CMake instead?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Note this will break OSX for now, so it should only be set for Linux (as it is done here).

So this change is doing the "Right Thing"[tm] but maybe accidentally?

See the tracking issue conda-forge/clang-compiler-activation-feedstock#17 for that.

Ack.

Can we remove the explicit setting of CMAKE_CXX_STANDARD in CMake instead?

That is way above my pay grade (for this project). It depends on whether arrow supports any compiler where the default C++ version is < C++11. For example, GCC defaults to C++98 until GCC 6.x, and there is a similar story with Clang.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@pitrou do you have any thoughts on whether we can change the default for CMAKE_CXX_STANDARD?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm. I'm not sure I understand the situation entirely, but for now Arrow C++ needs to compile on gcc 4.9 (and perhaps even gcc 4.8, for a subset of Arrow). This is because of the compiler requirements for R packages...

@pitroupitrouDec 13, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also, unless I misunderstand @xhochy 's first message ("it should only be set for Linux (as it is done here)"), it seems there's no real problem here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

My message was mainly a heads-up that on OSX, you need to set CMAKE_CXX_STANDARD to 14. This is Linux here, so everything is fine here.

I'm not sure about the general removal of CMAKE_CXX_STANDARD as google-cloud-cpp does also set it: https://github.com/googleapis/google-cloud-cpp/blob/13ec1e946ae1baad6bcae952daf5910649dcfd0a/CMakeLists.txt#L31-L41 Possibly that combination could also be used here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Well, we already have the following:

if(NOTDEFINEDCMAKE_CXX_STANDARD)
set(CMAKE_CXX_STANDARD 11)
endif()

The only difference AFAICT is that we don't error out if the user explicitly asked for something earlier than C++11, but that must be a really rare case.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@xhochy Can you remind me the reason of CMAKE_CXX_STANDARD=17 here? Also, perhaps explain it in a comment?

GTest_SOURCE=BUNDLED \
PARQUET_BUILD_EXAMPLES=ON \
PARQUET_BUILD_EXECUTABLES=ON \
Expand Down
1 change: 1 addition & 0 deletions ci/scripts/cpp_build.sh
Original file line numberDiff line numberDiff line change
Expand Up@@ -135,6 +135,7 @@ cmake \
-DCMAKE_BUILD_TYPE=${ARROW_BUILD_TYPE:-debug} \
-DCMAKE_C_FLAGS="${CFLAGS:-}" \
-DCMAKE_CXX_FLAGS="${CXXFLAGS:-}" \
-DCMAKE_CXX_STANDARD="${CMAKE_CXX_STANDARD:-11}" \
-DCMAKE_INSTALL_LIBDIR=${CMAKE_INSTALL_LIBDIR:-lib} \
-DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX:-${ARROW_HOME}} \
-DCMAKE_UNITY_BUILD=${CMAKE_UNITY_BUILD:-OFF} \
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/filesystem/gcsfs.cc
Original file line numberDiff line numberDiff line change
Expand Up@@ -138,7 +138,7 @@ class GcsInputStream : public arrow::io::InputStream {
stream_.read(reinterpret_cast<char*>(buffer->mutable_data()), nbytes);
ARROW_GCS_RETURN_NOT_OK(stream_.status());
RETURN_NOT_OK(buffer->Resize(stream_.gcount(), true));
return buffer;
return std::shared_ptr<Buffer>(std::move(buffer));
}
//@}

Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
aws_sdk_cpp:
- 1.8.186
bzip2:
- '1'
c_compiler:
- gcc
c_compiler_version:
- '9'
- '7'
cdt_name:
- cos6
channel_sources:
- conda-forge,defaults
- conda-forge
channel_targets:
- conda-forge main
cuda_compiler:
Expand All@@ -19,23 +17,25 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '9'
- '7'
docker_image:
- quay.io/condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cos7-cuda:10.2
gflags:
- '2.2'
glog:
- '0.5'
grpc_cpp:
- '1.38'
- '1.42'
libprotobuf:
- '3.16'
- '3.19'
lz4_c:
- 1.9.3
numpy:
- '1.17'
- '1.18'
openssl:
- 1.1.1
orc:
- 1.6.8
- 1.7.2
pin_run_as_build:
bzip2:
max_pin: x
Expand All@@ -49,17 +49,17 @@ pin_run_as_build:
python:
- 3.7.* *_cpython
re2:
- 2021.06.01
- 2021.11.01
snappy:
- '1'
target_platform:
- linux-64
thrift_cpp:
- 0.14.2
- 0.15.0
zip_keys:
- - c_compiler_version
- cxx_compiler_version
- - cuda_compiler_version
- cuda_compiler_version
- cdt_name
- docker_image
- - python
Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
aws_sdk_cpp:
- 1.8.186
bzip2:
- '1'
c_compiler:
- gcc
c_compiler_version:
- '9'
- '7'
cdt_name:
- cos6
channel_sources:
- conda-forge,defaults
- conda-forge
channel_targets:
- conda-forge main
cuda_compiler:
Expand All@@ -19,23 +17,25 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '9'
- '7'
docker_image:
- quay.io/condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cos7-cuda:10.2
gflags:
- '2.2'
glog:
- '0.5'
grpc_cpp:
- '1.38'
- '1.42'
libprotobuf:
- '3.16'
- '3.19'
lz4_c:
- 1.9.3
numpy:
- '1.17'
- '1.18'
openssl:
- 1.1.1
orc:
- 1.6.8
- 1.7.2
pin_run_as_build:
bzip2:
max_pin: x
Expand All@@ -49,17 +49,17 @@ pin_run_as_build:
python:
- 3.8.* *_cpython
re2:
- 2021.06.01
- 2021.11.01
snappy:
- '1'
target_platform:
- linux-64
thrift_cpp:
- 0.14.2
- 0.15.0
zip_keys:
- - c_compiler_version
- cxx_compiler_version
- - cuda_compiler_version
- cuda_compiler_version
- cdt_name
- docker_image
- - python
Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
aws_sdk_cpp:
- 1.8.186
bzip2:
- '1'
c_compiler:
- gcc
c_compiler_version:
- '9'
- '7'
cdt_name:
- cos6
channel_sources:
- conda-forge,defaults
- conda-forge
channel_targets:
- conda-forge main
cuda_compiler:
Expand All@@ -19,23 +17,25 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '9'
- '7'
docker_image:
- quay.io/condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cos7-cuda:10.2
gflags:
- '2.2'
glog:
- '0.5'
grpc_cpp:
- '1.38'
- '1.42'
libprotobuf:
- '3.16'
- '3.19'
lz4_c:
- 1.9.3
numpy:
- '1.19'
openssl:
- 1.1.1
orc:
- 1.6.8
- 1.7.2
pin_run_as_build:
bzip2:
max_pin: x
Expand All@@ -49,17 +49,17 @@ pin_run_as_build:
python:
- 3.9.* *_cpython
re2:
- 2021.06.01
- 2021.11.01
snappy:
- '1'
target_platform:
- linux-64
thrift_cpp:
- 0.14.2
- 0.15.0
zip_keys:
- - c_compiler_version
- cxx_compiler_version
- - cuda_compiler_version
- cuda_compiler_version
- cdt_name
- docker_image
- - python
Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
aws_sdk_cpp:
- 1.8.186
bzip2:
- '1'
c_compiler:
- gcc
c_compiler_version:
- '9'
- '7'
cdt_name:
- cos6
channel_sources:
- conda-forge,defaults
- conda-forge
channel_targets:
- conda-forge main
cuda_compiler:
Expand All@@ -19,23 +17,25 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '9'
- '7'
docker_image:
- quay.io/condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cos7-cuda:10.2
gflags:
- '2.2'
glog:
- '0.5'
grpc_cpp:
- '1.38'
- '1.42'
libprotobuf:
- '3.16'
- '3.19'
lz4_c:
- 1.9.3
numpy:
- '1.17'
- '1.21'
openssl:
- 1.1.1
orc:
- 1.6.8
- 1.7.2
pin_run_as_build:
bzip2:
max_pin: x
Expand All@@ -47,19 +47,19 @@ pin_run_as_build:
zlib:
max_pin: x.x
python:
- 3.6.* *_cpython
- 3.10.* *_cpython
re2:
- 2021.06.01
- 2021.11.01
snappy:
- '1'
target_platform:
- linux-64
thrift_cpp:
- 0.14.2
- 0.15.0
zip_keys:
- - c_compiler_version
- cxx_compiler_version
- - cuda_compiler_version
- cuda_compiler_version
- cdt_name
- docker_image
- - python
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion ci/conda_env_cpp.txt
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,7 +15,7 @@
# specific language governing permissions and limitations
# under the License.

aws-sdk-cpp
aws-sdk-cpp=1.8.186
benchmark>=1.6.0
boost-cpp>=1.68.0
brotli
Expand All@@ -25,13 +25,17 @@ cmake
gflags
glog
gmock>=1.10.0
google-cloud-cpp>=1.34.0
grpc-cpp>=1.27.3
gtest>=1.10.0
libprotobuf
libutf8proc
lz4-c
make
ninja
# Required by google-cloud-cpp, the Conda package is missing the dependency:
# https://github.com/conda-forge/google-cloud-cpp-feedstock/issues/28
nlohmann_json
pkg-config
python
rapidjson
Expand Down
9 changes: 6 additions & 3 deletions ci/docker/conda-cpp.dockerfile
Original file line numberDiff line numberDiff line change
Expand Up@@ -22,9 +22,6 @@ FROM ${repo}:${arch}-conda
COPY ci/scripts/install_minio.sh /arrow/ci/scripts
RUN /arrow/ci/scripts/install_minio.sh latest /opt/conda

COPY ci/scripts/install_gcs_testbench.sh /arrow/ci/scripts
RUN /arrow/ci/scripts/install_gcs_testbench.sh default

# install the required conda packages into the test environment
COPY ci/conda_env_cpp.txt \
ci/conda_env_gandiva.txt \
Expand All@@ -37,12 +34,17 @@ RUN mamba install \
valgrind && \
mamba clean --all

# We want to install the GCS testbench using the same Python binary that the Conda code will use.
COPY ci/scripts/install_gcs_testbench.sh /arrow/ci/scripts
RUN /arrow/ci/scripts/install_gcs_testbench.sh default

ENV ARROW_BUILD_TESTS=ON \
ARROW_DATASET=ON \
ARROW_DEPENDENCY_SOURCE=CONDA \
ARROW_FLIGHT=ON \
ARROW_FLIGHT_SQL=ON \
ARROW_GANDIVA=ON \
ARROW_GCS=ON \
ARROW_HOME=$CONDA_PREFIX \
ARROW_ORC=ON \
ARROW_PARQUET=ON \
Expand All@@ -55,6 +57,7 @@ ENV ARROW_BUILD_TESTS=ON \
ARROW_WITH_SNAPPY=ON \
ARROW_WITH_ZLIB=ON \
ARROW_WITH_ZSTD=ON \
CMAKE_CXX_STANDARD=17 \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Note this will break OSX for now, so it should only be set for Linux (as it is done here). See the tracking issue conda-forge/clang-compiler-activation-feedstock#17 for that. Can we remove the explicit setting of CMAKE_CXX_STANDARD in CMake instead?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Note this will break OSX for now, so it should only be set for Linux (as it is done here).

So this change is doing the "Right Thing"[tm] but maybe accidentally?

See the tracking issue conda-forge/clang-compiler-activation-feedstock#17 for that.

Ack.

Can we remove the explicit setting of CMAKE_CXX_STANDARD in CMake instead?

That is way above my pay grade (for this project). It depends on whether arrow supports any compiler where the default C++ version is < C++11. For example, GCC defaults to C++98 until GCC 6.x, and there is a similar story with Clang.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@pitrou do you have any thoughts on whether we can change the default for CMAKE_CXX_STANDARD?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm. I'm not sure I understand the situation entirely, but for now Arrow C++ needs to compile on gcc 4.9 (and perhaps even gcc 4.8, for a subset of Arrow). This is because of the compiler requirements for R packages...

@pitroupitrouDec 13, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also, unless I misunderstand @xhochy 's first message ("it should only be set for Linux (as it is done here)"), it seems there's no real problem here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

My message was mainly a heads-up that on OSX, you need to set CMAKE_CXX_STANDARD to 14. This is Linux here, so everything is fine here.

I'm not sure about the general removal of CMAKE_CXX_STANDARD as google-cloud-cpp does also set it: https://github.com/googleapis/google-cloud-cpp/blob/13ec1e946ae1baad6bcae952daf5910649dcfd0a/CMakeLists.txt#L31-L41 Possibly that combination could also be used here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Well, we already have the following:

if(NOTDEFINEDCMAKE_CXX_STANDARD)
set(CMAKE_CXX_STANDARD 11)
endif()

The only difference AFAICT is that we don't error out if the user explicitly asked for something earlier than C++11, but that must be a really rare case.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@xhochy Can you remind me the reason of CMAKE_CXX_STANDARD=17 here? Also, perhaps explain it in a comment?

GTest_SOURCE=BUNDLED \
PARQUET_BUILD_EXAMPLES=ON \
PARQUET_BUILD_EXECUTABLES=ON \
Expand Down
1 change: 1 addition & 0 deletions ci/scripts/cpp_build.sh
Original file line numberDiff line numberDiff line change
Expand Up@@ -135,6 +135,7 @@ cmake \
-DCMAKE_BUILD_TYPE=${ARROW_BUILD_TYPE:-debug} \
-DCMAKE_C_FLAGS="${CFLAGS:-}" \
-DCMAKE_CXX_FLAGS="${CXXFLAGS:-}" \
-DCMAKE_CXX_STANDARD="${CMAKE_CXX_STANDARD:-11}" \
-DCMAKE_INSTALL_LIBDIR=${CMAKE_INSTALL_LIBDIR:-lib} \
-DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX:-${ARROW_HOME}} \
-DCMAKE_UNITY_BUILD=${CMAKE_UNITY_BUILD:-OFF} \
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/filesystem/gcsfs.cc
Original file line numberDiff line numberDiff line change
Expand Up@@ -138,7 +138,7 @@ class GcsInputStream : public arrow::io::InputStream {
stream_.read(reinterpret_cast<char*>(buffer->mutable_data()), nbytes);
ARROW_GCS_RETURN_NOT_OK(stream_.status());
RETURN_NOT_OK(buffer->Resize(stream_.gcount(), true));
return buffer;
return std::shared_ptr<Buffer>(std::move(buffer));
}
//@}

Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
aws_sdk_cpp:
- 1.8.186
bzip2:
- '1'
c_compiler:
- gcc
c_compiler_version:
- '9'
- '7'
cdt_name:
- cos6
channel_sources:
- conda-forge,defaults
- conda-forge
channel_targets:
- conda-forge main
cuda_compiler:
Expand All@@ -19,23 +17,25 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '9'
- '7'
docker_image:
- quay.io/condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cos7-cuda:10.2
gflags:
- '2.2'
glog:
- '0.5'
grpc_cpp:
- '1.38'
- '1.42'
libprotobuf:
- '3.16'
- '3.19'
lz4_c:
- 1.9.3
numpy:
- '1.17'
- '1.18'
openssl:
- 1.1.1
orc:
- 1.6.8
- 1.7.2
pin_run_as_build:
bzip2:
max_pin: x
Expand All@@ -49,17 +49,17 @@ pin_run_as_build:
python:
- 3.7.* *_cpython
re2:
- 2021.06.01
- 2021.11.01
snappy:
- '1'
target_platform:
- linux-64
thrift_cpp:
- 0.14.2
- 0.15.0
zip_keys:
- - c_compiler_version
- cxx_compiler_version
- - cuda_compiler_version
- cuda_compiler_version
- cdt_name
- docker_image
- - python
Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
aws_sdk_cpp:
- 1.8.186
bzip2:
- '1'
c_compiler:
- gcc
c_compiler_version:
- '9'
- '7'
cdt_name:
- cos6
channel_sources:
- conda-forge,defaults
- conda-forge
channel_targets:
- conda-forge main
cuda_compiler:
Expand All@@ -19,23 +17,25 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '9'
- '7'
docker_image:
- quay.io/condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cos7-cuda:10.2
gflags:
- '2.2'
glog:
- '0.5'
grpc_cpp:
- '1.38'
- '1.42'
libprotobuf:
- '3.16'
- '3.19'
lz4_c:
- 1.9.3
numpy:
- '1.17'
- '1.18'
openssl:
- 1.1.1
orc:
- 1.6.8
- 1.7.2
pin_run_as_build:
bzip2:
max_pin: x
Expand All@@ -49,17 +49,17 @@ pin_run_as_build:
python:
- 3.8.* *_cpython
re2:
- 2021.06.01
- 2021.11.01
snappy:
- '1'
target_platform:
- linux-64
thrift_cpp:
- 0.14.2
- 0.15.0
zip_keys:
- - c_compiler_version
- cxx_compiler_version
- - cuda_compiler_version
- cuda_compiler_version
- cdt_name
- docker_image
- - python
Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
aws_sdk_cpp:
- 1.8.186
bzip2:
- '1'
c_compiler:
- gcc
c_compiler_version:
- '9'
- '7'
cdt_name:
- cos6
channel_sources:
- conda-forge,defaults
- conda-forge
channel_targets:
- conda-forge main
cuda_compiler:
Expand All@@ -19,23 +17,25 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '9'
- '7'
docker_image:
- quay.io/condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cos7-cuda:10.2
gflags:
- '2.2'
glog:
- '0.5'
grpc_cpp:
- '1.38'
- '1.42'
libprotobuf:
- '3.16'
- '3.19'
lz4_c:
- 1.9.3
numpy:
- '1.19'
openssl:
- 1.1.1
orc:
- 1.6.8
- 1.7.2
pin_run_as_build:
bzip2:
max_pin: x
Expand All@@ -49,17 +49,17 @@ pin_run_as_build:
python:
- 3.9.* *_cpython
re2:
- 2021.06.01
- 2021.11.01
snappy:
- '1'
target_platform:
- linux-64
thrift_cpp:
- 0.14.2
- 0.15.0
zip_keys:
- - c_compiler_version
- cxx_compiler_version
- - cuda_compiler_version
- cuda_compiler_version
- cdt_name
- docker_image
- - python
Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
aws_sdk_cpp:
- 1.8.186
bzip2:
- '1'
c_compiler:
- gcc
c_compiler_version:
- '9'
- '7'
cdt_name:
- cos6
channel_sources:
- conda-forge,defaults
- conda-forge
channel_targets:
- conda-forge main
cuda_compiler:
Expand All@@ -19,23 +17,25 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '9'
- '7'
docker_image:
- quay.io/condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cos7-cuda:10.2
gflags:
- '2.2'
glog:
- '0.5'
grpc_cpp:
- '1.38'
- '1.42'
libprotobuf:
- '3.16'
- '3.19'
lz4_c:
- 1.9.3
numpy:
- '1.17'
- '1.21'
openssl:
- 1.1.1
orc:
- 1.6.8
- 1.7.2
pin_run_as_build:
bzip2:
max_pin: x
Expand All@@ -47,19 +47,19 @@ pin_run_as_build:
zlib:
max_pin: x.x
python:
- 3.6.* *_cpython
- 3.10.* *_cpython
re2:
- 2021.06.01
- 2021.11.01
snappy:
- '1'
target_platform:
- linux-64
thrift_cpp:
- 0.14.2
- 0.15.0
zip_keys:
- - c_compiler_version
- cxx_compiler_version
- - cuda_compiler_version
- cuda_compiler_version
- cdt_name
- docker_image
- - python
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion ci/conda_env_cpp.txt
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,7 +15,7 @@
# specific language governing permissions and limitations
# under the License.

aws-sdk-cpp
aws-sdk-cpp=1.8.186
benchmark>=1.6.0
boost-cpp>=1.68.0
brotli
Expand All@@ -25,13 +25,17 @@ cmake
gflags
glog
gmock>=1.10.0
google-cloud-cpp>=1.34.0
grpc-cpp>=1.27.3
gtest>=1.10.0
libprotobuf
libutf8proc
lz4-c
make
ninja
# Required by google-cloud-cpp, the Conda package is missing the dependency:
# https://github.com/conda-forge/google-cloud-cpp-feedstock/issues/28
nlohmann_json
pkg-config
python
rapidjson
Expand Down
9 changes: 6 additions & 3 deletions ci/docker/conda-cpp.dockerfile
Original file line numberDiff line numberDiff line change
Expand Up@@ -22,9 +22,6 @@ FROM ${repo}:${arch}-conda
COPY ci/scripts/install_minio.sh /arrow/ci/scripts
RUN /arrow/ci/scripts/install_minio.sh latest /opt/conda

COPY ci/scripts/install_gcs_testbench.sh /arrow/ci/scripts
RUN /arrow/ci/scripts/install_gcs_testbench.sh default

# install the required conda packages into the test environment
COPY ci/conda_env_cpp.txt \
ci/conda_env_gandiva.txt \
Expand All@@ -37,12 +34,17 @@ RUN mamba install \
valgrind && \
mamba clean --all

# We want to install the GCS testbench using the same Python binary that the Conda code will use.
COPY ci/scripts/install_gcs_testbench.sh /arrow/ci/scripts
RUN /arrow/ci/scripts/install_gcs_testbench.sh default

ENV ARROW_BUILD_TESTS=ON \
ARROW_DATASET=ON \
ARROW_DEPENDENCY_SOURCE=CONDA \
ARROW_FLIGHT=ON \
ARROW_FLIGHT_SQL=ON \
ARROW_GANDIVA=ON \
ARROW_GCS=ON \
ARROW_HOME=$CONDA_PREFIX \
ARROW_ORC=ON \
ARROW_PARQUET=ON \
Expand All@@ -55,6 +57,7 @@ ENV ARROW_BUILD_TESTS=ON \
ARROW_WITH_SNAPPY=ON \
ARROW_WITH_ZLIB=ON \
ARROW_WITH_ZSTD=ON \
CMAKE_CXX_STANDARD=17 \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Note this will break OSX for now, so it should only be set for Linux (as it is done here). See the tracking issue conda-forge/clang-compiler-activation-feedstock#17 for that. Can we remove the explicit setting of CMAKE_CXX_STANDARD in CMake instead?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Note this will break OSX for now, so it should only be set for Linux (as it is done here).

So this change is doing the "Right Thing"[tm] but maybe accidentally?

See the tracking issue conda-forge/clang-compiler-activation-feedstock#17 for that.

Ack.

Can we remove the explicit setting of CMAKE_CXX_STANDARD in CMake instead?

That is way above my pay grade (for this project). It depends on whether arrow supports any compiler where the default C++ version is < C++11. For example, GCC defaults to C++98 until GCC 6.x, and there is a similar story with Clang.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@pitrou do you have any thoughts on whether we can change the default for CMAKE_CXX_STANDARD?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm. I'm not sure I understand the situation entirely, but for now Arrow C++ needs to compile on gcc 4.9 (and perhaps even gcc 4.8, for a subset of Arrow). This is because of the compiler requirements for R packages...

@pitroupitrouDec 13, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also, unless I misunderstand @xhochy 's first message ("it should only be set for Linux (as it is done here)"), it seems there's no real problem here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

My message was mainly a heads-up that on OSX, you need to set CMAKE_CXX_STANDARD to 14. This is Linux here, so everything is fine here.

I'm not sure about the general removal of CMAKE_CXX_STANDARD as google-cloud-cpp does also set it: https://github.com/googleapis/google-cloud-cpp/blob/13ec1e946ae1baad6bcae952daf5910649dcfd0a/CMakeLists.txt#L31-L41 Possibly that combination could also be used here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Well, we already have the following:

if(NOTDEFINEDCMAKE_CXX_STANDARD)
set(CMAKE_CXX_STANDARD 11)
endif()

The only difference AFAICT is that we don't error out if the user explicitly asked for something earlier than C++11, but that must be a really rare case.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@xhochy Can you remind me the reason of CMAKE_CXX_STANDARD=17 here? Also, perhaps explain it in a comment?

GTest_SOURCE=BUNDLED \
PARQUET_BUILD_EXAMPLES=ON \
PARQUET_BUILD_EXECUTABLES=ON \
Expand Down
1 change: 1 addition & 0 deletions ci/scripts/cpp_build.sh
Original file line numberDiff line numberDiff line change
Expand Up@@ -135,6 +135,7 @@ cmake \
-DCMAKE_BUILD_TYPE=${ARROW_BUILD_TYPE:-debug} \
-DCMAKE_C_FLAGS="${CFLAGS:-}" \
-DCMAKE_CXX_FLAGS="${CXXFLAGS:-}" \
-DCMAKE_CXX_STANDARD="${CMAKE_CXX_STANDARD:-11}" \
-DCMAKE_INSTALL_LIBDIR=${CMAKE_INSTALL_LIBDIR:-lib} \
-DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX:-${ARROW_HOME}} \
-DCMAKE_UNITY_BUILD=${CMAKE_UNITY_BUILD:-OFF} \
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/filesystem/gcsfs.cc
Original file line numberDiff line numberDiff line change
Expand Up@@ -138,7 +138,7 @@ class GcsInputStream : public arrow::io::InputStream {
stream_.read(reinterpret_cast<char*>(buffer->mutable_data()), nbytes);
ARROW_GCS_RETURN_NOT_OK(stream_.status());
RETURN_NOT_OK(buffer->Resize(stream_.gcount(), true));
return buffer;
return std::shared_ptr<Buffer>(std::move(buffer));
}
//@}

Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
aws_sdk_cpp:
- 1.8.186
bzip2:
- '1'
c_compiler:
- gcc
c_compiler_version:
- '9'
- '7'
cdt_name:
- cos6
channel_sources:
- conda-forge,defaults
- conda-forge
channel_targets:
- conda-forge main
cuda_compiler:
Expand All@@ -19,23 +17,25 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '9'
- '7'
docker_image:
- quay.io/condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cos7-cuda:10.2
gflags:
- '2.2'
glog:
- '0.5'
grpc_cpp:
- '1.38'
- '1.42'
libprotobuf:
- '3.16'
- '3.19'
lz4_c:
- 1.9.3
numpy:
- '1.17'
- '1.18'
openssl:
- 1.1.1
orc:
- 1.6.8
- 1.7.2
pin_run_as_build:
bzip2:
max_pin: x
Expand All@@ -49,17 +49,17 @@ pin_run_as_build:
python:
- 3.7.* *_cpython
re2:
- 2021.06.01
- 2021.11.01
snappy:
- '1'
target_platform:
- linux-64
thrift_cpp:
- 0.14.2
- 0.15.0
zip_keys:
- - c_compiler_version
- cxx_compiler_version
- - cuda_compiler_version
- cuda_compiler_version
- cdt_name
- docker_image
- - python
Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
aws_sdk_cpp:
- 1.8.186
bzip2:
- '1'
c_compiler:
- gcc
c_compiler_version:
- '9'
- '7'
cdt_name:
- cos6
channel_sources:
- conda-forge,defaults
- conda-forge
channel_targets:
- conda-forge main
cuda_compiler:
Expand All@@ -19,23 +17,25 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '9'
- '7'
docker_image:
- quay.io/condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cos7-cuda:10.2
gflags:
- '2.2'
glog:
- '0.5'
grpc_cpp:
- '1.38'
- '1.42'
libprotobuf:
- '3.16'
- '3.19'
lz4_c:
- 1.9.3
numpy:
- '1.17'
- '1.18'
openssl:
- 1.1.1
orc:
- 1.6.8
- 1.7.2
pin_run_as_build:
bzip2:
max_pin: x
Expand All@@ -49,17 +49,17 @@ pin_run_as_build:
python:
- 3.8.* *_cpython
re2:
- 2021.06.01
- 2021.11.01
snappy:
- '1'
target_platform:
- linux-64
thrift_cpp:
- 0.14.2
- 0.15.0
zip_keys:
- - c_compiler_version
- cxx_compiler_version
- - cuda_compiler_version
- cuda_compiler_version
- cdt_name
- docker_image
- - python
Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
aws_sdk_cpp:
- 1.8.186
bzip2:
- '1'
c_compiler:
- gcc
c_compiler_version:
- '9'
- '7'
cdt_name:
- cos6
channel_sources:
- conda-forge,defaults
- conda-forge
channel_targets:
- conda-forge main
cuda_compiler:
Expand All@@ -19,23 +17,25 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '9'
- '7'
docker_image:
- quay.io/condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cos7-cuda:10.2
gflags:
- '2.2'
glog:
- '0.5'
grpc_cpp:
- '1.38'
- '1.42'
libprotobuf:
- '3.16'
- '3.19'
lz4_c:
- 1.9.3
numpy:
- '1.19'
openssl:
- 1.1.1
orc:
- 1.6.8
- 1.7.2
pin_run_as_build:
bzip2:
max_pin: x
Expand All@@ -49,17 +49,17 @@ pin_run_as_build:
python:
- 3.9.* *_cpython
re2:
- 2021.06.01
- 2021.11.01
snappy:
- '1'
target_platform:
- linux-64
thrift_cpp:
- 0.14.2
- 0.15.0
zip_keys:
- - c_compiler_version
- cxx_compiler_version
- - cuda_compiler_version
- cuda_compiler_version
- cdt_name
- docker_image
- - python
Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
aws_sdk_cpp:
- 1.8.186
bzip2:
- '1'
c_compiler:
- gcc
c_compiler_version:
- '9'
- '7'
cdt_name:
- cos6
channel_sources:
- conda-forge,defaults
- conda-forge
channel_targets:
- conda-forge main
cuda_compiler:
Expand All@@ -19,23 +17,25 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '9'
- '7'
docker_image:
- quay.io/condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cos7-cuda:10.2
gflags:
- '2.2'
glog:
- '0.5'
grpc_cpp:
- '1.38'
- '1.42'
libprotobuf:
- '3.16'
- '3.19'
lz4_c:
- 1.9.3
numpy:
- '1.17'
- '1.21'
openssl:
- 1.1.1
orc:
- 1.6.8
- 1.7.2
pin_run_as_build:
bzip2:
max_pin: x
Expand All@@ -47,19 +47,19 @@ pin_run_as_build:
zlib:
max_pin: x.x
python:
- 3.6.* *_cpython
- 3.10.* *_cpython
re2:
- 2021.06.01
- 2021.11.01
snappy:
- '1'
target_platform:
- linux-64
thrift_cpp:
- 0.14.2
- 0.15.0
zip_keys:
- - c_compiler_version
- cxx_compiler_version
- - cuda_compiler_version
- cuda_compiler_version
- cdt_name
- docker_image
- - python
Expand Down
Loading