[C++] Place vendored double-conversion library in a private namespace #35134
Comments
mikelui
changed the title
kou
pushed a commit
that referenced
this issue
Apr 17, 2023
…on library (#35135) ### Rationale for this change See #35134 This change avoids potential symbol collisions when linking Arrow within other libraries. This actually seems like the original intention (a private namespace) per the following merge: 767c953 The message in that merge is confusing, because it implies that the merge made the library private, when it actually did the opposite (specifically in 3b89b19). A quote is below: > Also, make its use private, because of Windows DLL exports. ### What changes are included in this PR? * an `update.sh` script to * automatically pull a new version, * apply the namespace change, * apply a custom patch for seemingly Gandiva (15137e2) * this patch will break in future versions of double-conversion, but it is at least documented in the update script * the update script is run for the current version The script is based on: https://github.com/apache/arrow/blob/main/cpp/src/arrow/vendored/fast_float/update.sh ### Are these changes tested? Yes. Built and tests run ### Are there any user-facing changes? Only if users used to depend on using the internal vendored double-conversion libraries for their own code. In such a case, when they upgrade, they would have to specify the `arrow_vendored` namespace. * Closes: #35134 Authored-by: Mike Lui <mikelui@meta.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
liujiacheng777
pushed a commit
to LoongArch-Python/arrow
that referenced
this issue
May 11, 2023
…nversion library (apache#35135) ### Rationale for this change See apache#35134 This change avoids potential symbol collisions when linking Arrow within other libraries. This actually seems like the original intention (a private namespace) per the following merge: 767c953 The message in that merge is confusing, because it implies that the merge made the library private, when it actually did the opposite (specifically in 3b89b19). A quote is below: > Also, make its use private, because of Windows DLL exports. ### What changes are included in this PR? * an `update.sh` script to * automatically pull a new version, * apply the namespace change, * apply a custom patch for seemingly Gandiva (15137e2) * this patch will break in future versions of double-conversion, but it is at least documented in the update script * the update script is run for the current version The script is based on: https://github.com/apache/arrow/blob/main/cpp/src/arrow/vendored/fast_float/update.sh ### Are these changes tested? Yes. Built and tests run ### Are there any user-facing changes? Only if users used to depend on using the internal vendored double-conversion libraries for their own code. In such a case, when they upgrade, they would have to specify the `arrow_vendored` namespace. * Closes: apache#35134 Authored-by: Mike Lui <mikelui@meta.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
ArgusLi
pushed a commit
to Bit-Quill/arrow
that referenced
this issue
May 15, 2023
…nversion library (apache#35135) ### Rationale for this change See apache#35134 This change avoids potential symbol collisions when linking Arrow within other libraries. This actually seems like the original intention (a private namespace) per the following merge: 767c953 The message in that merge is confusing, because it implies that the merge made the library private, when it actually did the opposite (specifically in 3b89b19). A quote is below: > Also, make its use private, because of Windows DLL exports. ### What changes are included in this PR? * an `update.sh` script to * automatically pull a new version, * apply the namespace change, * apply a custom patch for seemingly Gandiva (15137e2) * this patch will break in future versions of double-conversion, but it is at least documented in the update script * the update script is run for the current version The script is based on: https://github.com/apache/arrow/blob/main/cpp/src/arrow/vendored/fast_float/update.sh ### Are these changes tested? Yes. Built and tests run ### Are there any user-facing changes? Only if users used to depend on using the internal vendored double-conversion libraries for their own code. In such a case, when they upgrade, they would have to specify the `arrow_vendored` namespace. * Closes: apache#35134 Authored-by: Mike Lui <mikelui@meta.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
rtpsw
pushed a commit
to rtpsw/arrow
that referenced
this issue
May 16, 2023
…nversion library (apache#35135) ### Rationale for this change See apache#35134 This change avoids potential symbol collisions when linking Arrow within other libraries. This actually seems like the original intention (a private namespace) per the following merge: 767c953 The message in that merge is confusing, because it implies that the merge made the library private, when it actually did the opposite (specifically in 3b89b19). A quote is below: > Also, make its use private, because of Windows DLL exports. ### What changes are included in this PR? * an `update.sh` script to * automatically pull a new version, * apply the namespace change, * apply a custom patch for seemingly Gandiva (15137e2) * this patch will break in future versions of double-conversion, but it is at least documented in the update script * the update script is run for the current version The script is based on: https://github.com/apache/arrow/blob/main/cpp/src/arrow/vendored/fast_float/update.sh ### Are these changes tested? Yes. Built and tests run ### Are there any user-facing changes? Only if users used to depend on using the internal vendored double-conversion libraries for their own code. In such a case, when they upgrade, they would have to specify the `arrow_vendored` namespace. * Closes: apache#35134 Authored-by: Mike Lui <mikelui@meta.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the enhancement requested
Double conversion is vendored in Arrow:
This can cause symbol collisions when the Arrow library is linked into other libraries which use a different version of double-conversion.
The vendored version should be placed into its own namespace to avoid these symbol collisions.
This is a sub-issue of #34932
Component(s)
C++
The text was updated successfully, but these errors were encountered: