You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "cxzl25 (via GitHub)" <gi...@apache.org> on 2024/04/10 12:28:03 UTC

[PR] [SPARK-47799][BUILD] Preserve parameter information when using SBT package jar [spark]

cxzl25 opened a new pull request, #45983:
URL: https://github.com/apache/spark/pull/45983

   ### What changes were proposed in this pull request?
   This PR aims to preserve parameter information when using SBT package jar.
   
   ### Why are the changes needed?
   `maven-compiler-plugin` defaults to debug=true, `plexus-compiler-javac` will add the parameter `-g`.
   
   SBT does not have this behavior by default, which leads to some differences between the jars of maven and sbt builds, although the code logic is the same.
   
   https://maven.apache.org/plugins/maven-compiler-plugin/compile-mojo.html#debug
   
   https://github.com/apache/maven-compiler-plugin/blob/736da68adf543cf56cd82a68e5ad28d397ace2f4/src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java#L734
   
   https://github.com/codehaus-plexus/plexus-compiler/blob/6ae79d7f2feca3a02e75f7661468abac6a9a0a11/plexus-compilers/plexus-compiler-javac/src/main/java/org/codehaus/plexus/compiler/javac/JavacCompiler.java#L279-L285
   
   
   ### Does this PR introduce _any_ user-facing change?
   No
   
   
   ### How was this patch tested?
   GA
   
   ```bash
   ./build/sbt common-utils/package
   ```
   
   local test
   
   ### Current
   
   <img width="1005" alt="image" src="https://github.com/apache/spark/assets/3898450/6233068c-b15b-46e4-a070-d3d36db540a4">
   
   
   ### PR
   <img width="1395" alt="image" src="https://github.com/apache/spark/assets/3898450/a5c156b1-d1a2-4847-9265-0c608a129091">
   
   
   
   ### Was this patch authored or co-authored using generative AI tooling?
   No
   


-- 
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@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-47799][BUILD] Add `-g` to javac compile parameters when using SBT package jar [spark]

Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on code in PR #45983:
URL: https://github.com/apache/spark/pull/45983#discussion_r1560761758


##########
project/SparkBuild.scala:
##########
@@ -311,6 +311,7 @@ object SparkBuild extends PomBuild {
 
     (Compile / javacOptions) ++= Seq(
       "-encoding", UTF_8.name(),
+      "-g",

Review Comment:
   >  we need to revise the PR title
   
   OK
   
   > but it's still smaller than Maven-generated size
   
   In this PR, the main difference between the two is the META-INF folder, maven generated more information, but SBT did not.
   
   
   <img width="251" alt="image" src="https://github.com/apache/spark/assets/3898450/8e2466ac-9962-4c96-a4e5-70cf59bf9dc3">
   
   
   <img width="1017" alt="image" src="https://github.com/apache/spark/assets/3898450/28d038cd-5949-485a-bf9d-0c8b1edbfdfe">
   



-- 
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@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-47799][BUILD] Add `-g` to javac compile parameters when using SBT package jar [spark]

Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on PR #45983:
URL: https://github.com/apache/spark/pull/45983#issuecomment-2050882315

   Thanks dongjoon.
   
   > which decompiler did you use for you screenshot
   
   I use jd-gui.
   https://github.com/java-decompiler/jd-gui


-- 
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@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-47799][BUILD] Add `-g` to javac compile parameters when using SBT package jar [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun closed pull request #45983: [SPARK-47799][BUILD] Add `-g` to javac compile parameters when using SBT package jar
URL: https://github.com/apache/spark/pull/45983


-- 
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@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-47799][BUILD] Add `-g` to javac compile parameters when using SBT package jar [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #45983:
URL: https://github.com/apache/spark/pull/45983#issuecomment-2050905018

   Thank you for the info. :)


-- 
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@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-47799][BUILD] Preserve parameter information when using SBT package jar [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #45983:
URL: https://github.com/apache/spark/pull/45983#discussion_r1559891702


##########
project/SparkBuild.scala:
##########
@@ -311,6 +311,7 @@ object SparkBuild extends PomBuild {
 
     (Compile / javacOptions) ++= Seq(
       "-encoding", UTF_8.name(),
+      "-g",

Review Comment:
   BTW, this is bigger than the PR title.
   > Preserve parameter information when using SBT package jar
   
   1. If this is only for parameter info, `-parameters` might be better.
   2. However, to be consistent Maven, `-g` is correct. 
   
   Given (2), I guess we need to revise the PR title, @cxzl25 .



##########
project/SparkBuild.scala:
##########
@@ -311,6 +311,7 @@ object SparkBuild extends PomBuild {
 
     (Compile / javacOptions) ++= Seq(
       "-encoding", UTF_8.name(),
+      "-g",

Review Comment:
   BTW, this is bigger than the PR title's scope
   > Preserve parameter information when using SBT package jar
   
   1. If this is only for parameter info, `-parameters` might be better.
   2. However, to be consistent Maven, `-g` is correct. 
   
   Given (2), I guess we need to revise the PR title, @cxzl25 .



-- 
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@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-47799][BUILD] Preserve parameter information when using SBT package jar [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #45983:
URL: https://github.com/apache/spark/pull/45983#discussion_r1559881657


##########
project/SparkBuild.scala:
##########
@@ -311,6 +311,7 @@ object SparkBuild extends PomBuild {
 
     (Compile / javacOptions) ++= Seq(
       "-encoding", UTF_8.name(),
+      "-g",

Review Comment:
   This looks safe to me. BTW, we still have differences, don't we?
   > SBT does not have this behavior by default, which leads to some differences between the jars of maven and sbt builds, 
   
   The file size seems to be increased from 386053 to 387076 with this, but it's still smaller than Maven-generated size, `397996`.



-- 
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@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org