Copilot commented on code in PR #9242: URL: https://github.com/apache/ozone/pull/9242#discussion_r2487906618
########## hadoop-hdds/docs/content/start/FromSource.md: ########## @@ -66,30 +66,28 @@ mvn install:install-file -DgroupId=com.google.protobuf -DartifactId=protoc -Dver cp $HOME/.m2/repository/com/google/protobuf/protoc/2.5.0/protoc-2.5.0-linux-aarch_64 $HOME/.m2/repository/com/google/protobuf/protoc/2.5.0/protoc-2.5.0-linux-aarch_64.exe ``` -## ARM-based Apple Silicon (Apple M1 ... etc) +## ARM-based Apple silicon (Apple M1, M2, M3, M4, M5, etc.) ```bash # Patch protobuf 2.5.0 - this is needed for Hadoop 2 support -PROTOBUF_VERSION="2.5.0" curl -LO https://github.com/protocolbuffers/protobuf/releases/download/v2.5.0/protobuf-2.5.0.tar.gz tar xzf protobuf-2.5.0.tar.gz -cd protobuf-${PROTOBUF_VERSION} - -# Open the file `src/google/protobuf/stubs/platform_macros.h` with an editor like vim and append the following lines after line 59 (include the #). -# Save the file when complete. +cd protobuf-2.5.0 -#elif defined(__arm64__) -#define GOOGLE_PROTOBUF_ARCH_ARM 1 -#define GOOGLE_PROTOBUF_ARCH_64_BIT 1 +# Insert arm64 macro to src/google/protobuf/stubs/platform_macros.h +sed -i '' '59 a\ +#elif defined(__arm64__)\ +#define GOOGLE_PROTOBUF_ARCH_ARM 1\ +#define GOOGLE_PROTOBUF_ARCH_64_BIT 1' src/google/protobuf/stubs/platform_macros.h Review Comment: The `sed -i ''` syntax is macOS-specific (BSD sed). This command will fail on Linux systems which use GNU sed and require `sed -i` without the empty string. Since this section is specifically for Apple Silicon, this is acceptable, but consider adding a comment clarifying this is macOS-only or providing an alternative for users who might try this on Linux ARM systems. ########## hadoop-hdds/docs/content/start/FromSource.md: ########## @@ -66,30 +66,28 @@ mvn install:install-file -DgroupId=com.google.protobuf -DartifactId=protoc -Dver cp $HOME/.m2/repository/com/google/protobuf/protoc/2.5.0/protoc-2.5.0-linux-aarch_64 $HOME/.m2/repository/com/google/protobuf/protoc/2.5.0/protoc-2.5.0-linux-aarch_64.exe ``` -## ARM-based Apple Silicon (Apple M1 ... etc) +## ARM-based Apple silicon (Apple M1, M2, M3, M4, M5, etc.) ```bash # Patch protobuf 2.5.0 - this is needed for Hadoop 2 support -PROTOBUF_VERSION="2.5.0" curl -LO https://github.com/protocolbuffers/protobuf/releases/download/v2.5.0/protobuf-2.5.0.tar.gz tar xzf protobuf-2.5.0.tar.gz -cd protobuf-${PROTOBUF_VERSION} - -# Open the file `src/google/protobuf/stubs/platform_macros.h` with an editor like vim and append the following lines after line 59 (include the #). -# Save the file when complete. +cd protobuf-2.5.0 -#elif defined(__arm64__) -#define GOOGLE_PROTOBUF_ARCH_ARM 1 -#define GOOGLE_PROTOBUF_ARCH_64_BIT 1 +# Insert arm64 macro to src/google/protobuf/stubs/platform_macros.h +sed -i '' '59 a\ +#elif defined(__arm64__)\ +#define GOOGLE_PROTOBUF_ARCH_ARM 1\ +#define GOOGLE_PROTOBUF_ARCH_64_BIT 1' src/google/protobuf/stubs/platform_macros.h # Execute the following commands to build `protoc` ./configure --disable-shared make -j -cd .. + Review Comment: The `cd ..` command was removed but there's no corresponding change back to the original directory before running `mvn install:install-file`. The `mvn` command references `-Dfile=src/protoc` which is a relative path from the `protobuf-2.5.0` directory. This command should either include `cd ..` before it or update the path to `-Dfile=protobuf-2.5.0/src/protoc`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
