You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pegasus.apache.org by "GehaFearless (via GitHub)" <gi...@apache.org> on 2023/02/16 13:59:50 UTC

[GitHub] [incubator-pegasus] GehaFearless opened a new pull request, #1356: fix: fix centos image build

GehaFearless opened a new pull request, #1356:
URL: https://github.com/apache/incubator-pegasus/pull/1356

   issue: https://github.com/apache/incubator-pegasus/issues/1347
   
   this pr fix centos6 & centos7 firstly.


-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] GehaFearless commented on a diff in pull request #1356: fix: fix centos image build

Posted by "GehaFearless (via GitHub)" <gi...@apache.org>.
GehaFearless commented on code in PR #1356:
URL: https://github.com/apache/incubator-pegasus/pull/1356#discussion_r1118390471


##########
docker/pegasus-build-env/centos6/Dockerfile:
##########
@@ -76,12 +76,6 @@ ENV MAVEN_HOME=/opt/rh/rh-maven33/root/usr/
 ENV PATH=$MAVEN_HOME/bin:$JAVA_HOME/bin:$GCC_HOME/bin:$PYTHON2_HOME/bin/:$PATH
 ENV LD_LIBRARY_PATH=$JAVA_HOME/jre/lib/amd64/server:$PYTHON2_HOME/lib64/:$LD_LIBRARY_PATH
 
-RUN pip install --no-cache-dir cmake
-
-RUN wget --progress=dot:giga https://github.com/apache/thrift/archive/refs/tags/0.11.0.tar.gz -P /opt/thrift && \

Review Comment:
   thrift download and build on thirdparty, not here



-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] levy5307 merged pull request #1356: fix: fix centos image build

Posted by "levy5307 (via GitHub)" <gi...@apache.org>.
levy5307 merged PR #1356:
URL: https://github.com/apache/incubator-pegasus/pull/1356


-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] acelyc111 commented on a diff in pull request #1356: fix: fix centos image build

Posted by "acelyc111 (via GitHub)" <gi...@apache.org>.
acelyc111 commented on code in PR #1356:
URL: https://github.com/apache/incubator-pegasus/pull/1356#discussion_r1118437478


##########
docker/pegasus-build-env/centos6/Dockerfile:
##########
@@ -76,12 +76,6 @@ ENV MAVEN_HOME=/opt/rh/rh-maven33/root/usr/
 ENV PATH=$MAVEN_HOME/bin:$JAVA_HOME/bin:$GCC_HOME/bin:$PYTHON2_HOME/bin/:$PATH
 ENV LD_LIBRARY_PATH=$JAVA_HOME/jre/lib/amd64/server:$PYTHON2_HOME/lib64/:$LD_LIBRARY_PATH
 
-RUN pip install --no-cache-dir cmake
-
-RUN wget --progress=dot:giga https://github.com/apache/thrift/archive/refs/tags/0.11.0.tar.gz -P /opt/thrift && \

Review Comment:
   This patch added these code: https://github.com/apache/incubator-pegasus/pull/1045
   
   That's because cpp code link thrift in version 0.9.3 (build by thirdparty), java code use thrift 0.11.0 binary (download here) to generate code from *.thrift files. Although it's confused, we can improve the different version lib/bin in another patch, but it would better to keep the logic in this patch.



-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] GehaFearless commented on a diff in pull request #1356: fix: fix centos image build

Posted by "GehaFearless (via GitHub)" <gi...@apache.org>.
GehaFearless commented on code in PR #1356:
URL: https://github.com/apache/incubator-pegasus/pull/1356#discussion_r1119911294


##########
docker/pegasus-build-env/centos6/Dockerfile:
##########
@@ -76,12 +76,6 @@ ENV MAVEN_HOME=/opt/rh/rh-maven33/root/usr/
 ENV PATH=$MAVEN_HOME/bin:$JAVA_HOME/bin:$GCC_HOME/bin:$PYTHON2_HOME/bin/:$PATH
 ENV LD_LIBRARY_PATH=$JAVA_HOME/jre/lib/amd64/server:$PYTHON2_HOME/lib64/:$LD_LIBRARY_PATH
 
-RUN pip install --no-cache-dir cmake
-
-RUN wget --progress=dot:giga https://github.com/apache/thrift/archive/refs/tags/0.11.0.tar.gz -P /opt/thrift && \

Review Comment:
   Right, I fix centos7 firstly



-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] acelyc111 commented on a diff in pull request #1356: fix: fix centos image build

Posted by "acelyc111 (via GitHub)" <gi...@apache.org>.
acelyc111 commented on code in PR #1356:
URL: https://github.com/apache/incubator-pegasus/pull/1356#discussion_r1112720090


##########
docker/pegasus-build-env/centos6/Dockerfile:
##########
@@ -76,12 +76,6 @@ ENV MAVEN_HOME=/opt/rh/rh-maven33/root/usr/
 ENV PATH=$MAVEN_HOME/bin:$JAVA_HOME/bin:$GCC_HOME/bin:$PYTHON2_HOME/bin/:$PATH
 ENV LD_LIBRARY_PATH=$JAVA_HOME/jre/lib/amd64/server:$PYTHON2_HOME/lib64/:$LD_LIBRARY_PATH
 
-RUN pip install --no-cache-dir cmake
-
-RUN wget --progress=dot:giga https://github.com/apache/thrift/archive/refs/tags/0.11.0.tar.gz -P /opt/thrift && \

Review Comment:
   thrift is used to generate cpp/java code from IDL, why removed it?



##########
docker/pegasus-build-env/centos7/Dockerfile:
##########
@@ -54,21 +54,21 @@ RUN yum -y install centos-release-scl \
                    yum clean all; \
                    rm -rf /var/cache/yum;
 
-RUN pip3 install --no-cache-dir cmake
+RUN pip3 install --upgrade pip --no-cache-dir && pip3 install --no-cache-dir cmake
 
 RUN wget --progress=dot:giga https://archive.apache.org/dist/maven/maven-3/3.8.3/binaries/apache-maven-3.8.3-bin.tar.gz -P /opt/maven \
     && cd /opt/maven \
     && tar -zxf apache-maven-3.8.3-bin.tar.gz \
     && rm apache-maven-3.8.3-bin.tar.gz
 
+ENV JAVA_HOME=/usr/lib/jvm/java-1.8.0-openjdk

Review Comment:
   What issue to solve by reordering?



-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org