You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by GitBox <gi...@apache.org> on 2021/09/22 02:54:36 UTC

[GitHub] [iotdb] jamber001 opened a new pull request #4007: [IOTDB-1718] correct the position of "chmod + cmake"

jamber001 opened a new pull request #4007:
URL: https://github.com/apache/iotdb/pull/4007


   [IOTDB-1718]  compile-tools/thrift/pom.xml, make-cmake-executable use wrong phase
   
   compile-tools/thrift/pom.xml:62
   
   ==============================
   <plugin>
   <groupId>org.codehaus.mojo</groupId>
   <artifactId>exec-maven-plugin</artifactId>
   <version>1.6.0</version>
   <executions>
   <execution>
   <id>make-cmake-executable</id>
   <phase>process-sources</phase> //here, this phase is too late, becasue "cmake-generate" begin to run cmake file in <phase>generate-sources</phase>
   <goals>
   <goal>exec</goal>
   </goals>
   <configuration>
   <basedir>${cmake.root.dir}/bin</basedir>
   <executable>${chmod.command}</executable>
   <commandlineArgs>${chmod.command.para}</commandlineArgs>
   </configuration>
   </execution>
   </executions>
   </plugin>
   ======================================
   
    "chmod +x cmake" should be run before running cmake.


-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] HTHou commented on pull request #4007: [IOTDB-1718] correct the position of "chmod + cmake"

Posted by GitBox <gi...@apache.org>.
HTHou commented on pull request #4007:
URL: https://github.com/apache/iotdb/pull/4007#issuecomment-926307091


   > @HTHou Can this PR be merged ?
   
   Sorry, I don't get the reason why this change can solve the problem you mentioned. Can you plz explain more?


-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] HTHou merged pull request #4007: [IOTDB-1718] correct the position of "chmod + cmake"

Posted by GitBox <gi...@apache.org>.
HTHou merged pull request #4007:
URL: https://github.com/apache/iotdb/pull/4007


   


-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] jamber001 commented on pull request #4007: [IOTDB-1718] correct the position of "chmod + cmake"

Posted by GitBox <gi...@apache.org>.
jamber001 commented on pull request #4007:
URL: https://github.com/apache/iotdb/pull/4007#issuecomment-926280656


   @HTHou    Can this PR  be merged ?


-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] jamber001 edited a comment on pull request #4007: [IOTDB-1718] correct the position of "chmod + cmake"

Posted by GitBox <gi...@apache.org>.
jamber001 edited a comment on pull request #4007:
URL: https://github.com/apache/iotdb/pull/4007#issuecomment-926373461


   > > @HTHou Can this PR be merged ?
   > 
   > Sorry, I don't get the reason why this change can solve the problem you mentioned. Can you plz explain more?
   
   I  try to give 1 summary for this part.  :-)
   In orginal IoTDB maven design, client-cpp's compiling procedure is as below:
   1) download cmake-3.17.3-Linux-x86_64.tar.gz and untar it.  (in maven phase <**generate-sources**>)
   2) download thrif 0.14.1 src code tar and untar it  (in maven phase <**generate-sources**>)
   3) use cmake to process thrift 0.14.1 src codes. (in maven phase <**generate-sources**>)
   4)  chmod +x cmake     (in maven phase <**process-sources**>)
   5) compile thrift 0.14.1  src code.  (in maven phase <**compile**>)  
   6)  more thrift generating work.   
   
   so, in above procedure, the step-4's postion should be 1 bug. 
   I guess that step-4 's orginal goal is to avoid new-downloaded cmake file has no executable permission, and step-4 should be run before step-3.
   But in origin codes, the step-4 was put in wrongly maven phase **process-sources**,  which casues it is run after step-3.
   
   **Note:** Of course, if the new-downloaded cmake file is originally executable, the step-4 is useless.
   
   My codes change is that:
   1) change the step-4's maven phase to **generate-sources**
   2) adjust step-4's position in pom.xml.
   so let "chmod +x cmake"  happen just after step-2. 
   


-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] coveralls commented on pull request #4007: [IOTDB-1718] correct the position of "chmod + cmake"

Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #4007:
URL: https://github.com/apache/iotdb/pull/4007#issuecomment-924555947


   
   [![Coverage Status](https://coveralls.io/builds/42982888/badge)](https://coveralls.io/builds/42982888)
   
   Coverage decreased (-0.03%) to 67.398% when pulling **f40bd92a19b611db6d61502f03f2e4e4e24d9652 on jamber001:IOTDB-1718** into **c75a71f76c10a3f79b905df3daeef5429704a7f7 on apache:master**.
   


-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] jamber001 commented on pull request #4007: [IOTDB-1718] correct the position of "chmod + cmake"

Posted by GitBox <gi...@apache.org>.
jamber001 commented on pull request #4007:
URL: https://github.com/apache/iotdb/pull/4007#issuecomment-926373461


   > > @HTHou Can this PR be merged ?
   > 
   > Sorry, I don't get the reason why this change can solve the problem you mentioned. Can you plz explain more?
   
   I  try to give 1 summary for this part.  :-)
   In orginal IoTDB maven design, client-cpp's compiling procedure is as below:
   1) download cmake-3.17.3-Linux-x86_64.tar.gz and untar it.  (in maven phase <**generate-sources**>)
   2) download thrif 0.14.1 src code tar and untar it  (in maven phase <**generate-sources**>)
   3) use cmake to process thrift 0.14.1 src codes. (in maven phase <**generate-sources**>)
   4)  chmod +x cmake     (in maven phase <**process-sources**>)
   5) compile thrift 0.14.1  src code.  (in maven phase <**compile**>)  
   6)  more thrift generating work.   
   
   so, in above procedure, the step-4's postion should be 1 bug. 
   I guess that step-4 's orginal goal is to avoid new-downloaded cmake file has no executable permission, and step-4 should be run before step-3.
   But in origin codes, the step-4 was put in wrongly maven phase **process-sources**,  which casues it is run after step-3.
   
   **Note:** Of course, if the new-downloaded cmake file is origianly executable, the step-4 is uesless.
   
   My codes change is that:
   1) change the step-4's maven phase to **generate-sources**
   2) adjust step-4's position in pom.xml.
   so let "chmod +x cmake"  happen just after step-2. 
   


-- 
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: reviews-unsubscribe@iotdb.apache.org

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