HDFS-15740. Add x-platform utilities #2567
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
GauthamBanasandra
force-pushed
the
rem-libgen
branch
from
67156ca to
c83cfe8
Compare
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
(!) A patch to the testing environment has been detected. |
|
💔 -1 overall
This message was automatically generated. |
|
(!) A patch to the testing environment has been detected. |
|
💔 -1 overall
This message was automatically generated. |
GauthamBanasandra
force-pushed
the
rem-libgen
branch
from
f446639 to
a20fbc8
Compare
|
(!) A patch to the testing environment has been detected. |
|
💔 -1 overall
This message was automatically generated. |
GauthamBanasandra
force-pushed
the
rem-libgen
branch
from
a20fbc8 to
ebedf29
Compare
|
(!) A patch to the testing environment has been detected. |
|
💔 -1 overall
This message was automatically generated. |
GauthamBanasandra
force-pushed
the
rem-libgen
branch
from
ebedf29 to
f7fa59f
Compare
|
(!) A patch to the testing environment has been detected. |
|
💔 -1 overall
This message was automatically generated. |
|
(!) A patch to the testing environment has been detected. |
|
💔 -1 overall
This message was automatically generated. |
|
(!) A patch to the testing environment has been detected. |
|
💔 -1 overall
This message was automatically generated. |
GauthamBanasandra
force-pushed
the
rem-libgen
branch
from
c791eac to
8d9333c
Compare
|
(!) A patch to the testing environment has been detected. |
|
💔 -1 overall
This message was automatically generated. |
* Had missed renaming target name. Fixing it now.
* bindings_c_obj doesn't link with x_platform_utils_obj through object linking. Thus, need to use dynamic linking.
* Object libraries can not be linked with or linked to. Thus, removing it.
* This upgrade is necessary for this PR as it enables x_platform_utils_obj to be used as part of the bindings_c_obj object library. This isn't supported in CMake 3.10 and the compilation would fail in the configuration step.
* Need to use GCC 9 since this PR uses C++17 features.
* Adding unit tests for x-platform/utils.
* Separated the test files for the appropriate platforms.
This reverts commit 8587ccdc988487069936789588566b84710af67c.
This reverts commit fa36f845911dfebb205aca42b077cc08006a91f4.
GauthamBanasandra
force-pushed
the
rem-libgen
branch
from
edecd13 to
eea4448
Compare
|
💔 -1 overall
This message was automatically generated. |
|
Would you update the GCC and CMake versions in BUILDING.txt? |
* Updated the BUILDING.txt document regarding GCC and CMake versions.
* Updated the CMake version required for Windows.
|
@aajisaka I've updated the BUILDING.txt now. Could you please have another look? |
goiri
approved these changes
Jan 27, 2021
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
@aajisaka Could you please review my PR? |
|
I think this is safe enough right now. |
|
@GauthamBanasandra @goiri Sorry for coming here late. This complicates many working Linux environment just for basename function. Can you make this optional profile for Windows only? |
|
@iwasakims the crux of my PR was to replace Linux specific code with a cross platform equivalent API so that Hadoop can run natively on all platforms. I've implemented the functionality of May I know what Linux specific complications you're referring to, that my PR doesn't address? I would be happy to address them in a way that's cross-platform friendly. 😊 |
gcc and cmake provided as OS standard package has been able to be used on distro such as CentOS 7 and 8, Debian 9 and 10 Ubuntu 16.04 and 18.04 befor this PR. We need to install gcc and cmake from source code or supplement package (if available) on most of those distros now. Just making basename portable is not worth for the hustle. How about just implementint the basename no Hadoop side? |
Actually @iwasakims , I'll be replacing all the Linux specific code in my further PRs. So, this upgrade to Focal serves as a foundation for my further PRs and isn't only for basename.
It isn't straightforward to do so. Also, it'll be error prone. I would rather rely on the implementation provided by C++17 standard library. I'm sure the C++ library authors write better code than me. 😁
Well, sooner or later, somebody would've upgraded to Focal and this would've been a problem anyway. I'm sure this was a problem at the time when the move from Xenial to Bionic happened. I'm not sure if it's any different now. |
Upgrading the Dockerfile to Focal did not change building requirements. It does not mean we do not need to care about platform other than Focal. You changed the requirements for your purpose here. Could you elaborate how to setup (CMake >= 3.19 and GCC >= 9) in the BUILDING.txt? |
|
@iwasakims I've added the steps to install CMake 3.19 and GCC 9.3.0 for Ubuntu 14.04 and Centos 8 - #2663 |
|
Thanks, @GauthamBanasandra. I added trivial review comment there. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It is a collection of components
that will help in writing cross
platform code.
upgrades - upgraded CMake to
3.19, GCC to 9 and C++ language
standard to 17.