Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade all dependencies to the latest versions #230

Merged
merged 12 commits into from
Oct 23, 2023

Conversation

Mingun
Copy link
Contributor

@Mingun Mingun commented Dec 5, 2020

@generalmimon
Copy link
Member

The commit d10b795 has been apparently pulled here by accident, it belongs to #222.

@Mingun Mingun force-pushed the upgrade-dependencies branch from a7ea6e6 to 687b427 Compare May 13, 2021 19:43
@Mingun Mingun marked this pull request as draft May 13, 2021 19:44
@Mingun
Copy link
Contributor Author

Mingun commented May 13, 2021

No, this branch is intentionally based on the #222 branch.

Not checked yet that project is in a buildable state after each commit after rebase, so converted to draft.

@Mingun Mingun force-pushed the upgrade-dependencies branch 2 times, most recently from 37e724a to 27f5980 Compare May 14, 2021 20:32
@Mingun Mingun marked this pull request as ready for review May 14, 2021 20:44
@Mingun Mingun force-pushed the upgrade-dependencies branch 4 times, most recently from 973ecc5 to c1066e2 Compare August 25, 2021 10:34
@Mingun
Copy link
Contributor Author

Mingun commented Aug 25, 2021

AS time goes it becomes more and more difficult to monitor the relevance of this PR. If you are not ready to take it all at once, then you can cherry-pick individual commits. The two scala-test commits are independent from the others, SnakeYaml and FastParse upgrades are also isolated and easy-testable.

The most significant upgrade is the scala-js because I have no ideas how to test it -- building kaitai-struct from sources still a complicated and not-well documented task and I have no time to learn it again. At least I tried to collect all useful links about changes in the each component -- see the corresponding commit messages.

Upgrading sbt-related infrastructure should be done in the commit order, because each step depending on the previous.

@Mingun Mingun force-pushed the upgrade-dependencies branch from c1066e2 to 1b1bdcf Compare August 11, 2022 18:17
@Mingun
Copy link
Contributor Author

Mingun commented Aug 11, 2022

Just note, that scala-js up to 1.10 has a security vulnerability and it is recommended to upgrade ASAP:
https://www.scala-js.org/news/2022/04/04/announcing-scalajs-1.10.0/

"com.lihaoyi" %%% "fastparse" % "1.0.0",
"org.yaml" % "snakeyaml" % "1.28"
"com.github.scopt" %%% "scopt" % "4.1.0",
"com.lihaoyi" %%% "fastparse" % "2.3.3",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noting that the format of FastParse error messages has changed, so when inserted to our own error message, we often end up with things like expected Expected .... See the comparison of sbt test output before and after this PR:

@@ -220,7 +220,7 @@ tr
 [info]         error: unable to access 'bar' in expr_field_unknown context (SimpleMatchers.scala:34)
 [info] - expr_inst_value_broken *** FAILED ***
 [info]   ..\tests\formats_err\expr_inst_value_broken.ksy: /instances/foo:
-[info]         error: parsing expression '1 *' failed on 1:3, expected "or" | CharsWhile(Set( , n)) | "\\\n" | End
+[info]         error: parsing expression '1 *' failed on 1:3, expected Expected end-of-input:1:3, found "*"
 [info]    did not equal expr_inst_value_broken.ksy: /instances/foo:
 [info]         error: parsing expression '1 *' failed on 1:3, expected "or" | CharsWhile(Set( , n)) | "\\\n" | End (SimpleMatchers.scala:34)
 [info] - expr_size_bad_id *** FAILED ***
@@ -258,7 +258,7 @@ tr
 [info]         error: unable to find type 'bar', searching from expr_type_cast_unknown (SimpleMatchers.scala:34)
 [info] - expr_unbalanced *** FAILED ***
 [info]   ..\tests\formats_err\expr_unbalanced.ksy: /seq/0:
-[info]         error: parsing expression '(1 + 5' failed on 1:1, expected "not" ~ !(namePart) ~ not_test | comparison
+[info]         error: parsing expression '(1 + 5' failed on 1:1, expected Expected (kw | comparison):1:1, found "(1 + 5"
 [info]    did not equal expr_unbalanced.ksy: /seq/0:
 [info]         error: parsing expression '(1 + 5' failed on 1:1, expected "not" ~ !(namePart) ~ not_test | comparison (SimpleMatchers.scala:34)
 [info] - expr_unknown_method1 *** FAILED ***
@@ -273,7 +273,7 @@ tr
 [info]         error: don't know how to call method 'frobnicate' of object type 'CalcBytesType' (SimpleMatchers.scala:34)
 [info] - expr_wrong_and *** FAILED ***
 [info]   ..\tests\formats_err\expr_wrong_and.ksy: /instances/both:
-[info]         error: parsing expression 'foo == 1 && bar == 2' failed on 1:10, expected "or" | CharsWhile(Set( , n)) | "\\\n" | End, did you mean 'and'?
+[info]         error: parsing expression 'foo == 1 && bar == 2' failed on 1:10, expected Expected end-of-input:1:10, found "&& bar == ", did you mean 'and'?
 [info]    did not equal expr_wrong_and.ksy: /instances/both:
 [info]         error: parsing expression 'foo == 1 && bar == 2' failed on 1:10, expected "or" | CharsWhile(Set( , n)) | "\\\n" | End, did you mean 'and'? (SimpleMatchers.scala:34)
 [info] - instance_name_bad *** FAILED ***
@@ -504,7 +504,7 @@ tr
 [info]         error: expected map, got foo (class java.lang.String) (SimpleMatchers.scala:34)
 [info] - switch_cases_malformed_quoting *** FAILED ***
 [info]   ..\tests\formats_err\switch_cases_malformed_quoting.ksy: /seq/1/cases/^AHEM:
-[info]         error: parsing expression '^AHEM' failed on 1:1, expected "not" ~ !(namePart) ~ not_test | comparison
+[info]         error: parsing expression '^AHEM' failed on 1:1, expected Expected (kw | comparison):1:1, found "^AHEM"
 [info]    did not equal switch_cases_malformed_quoting.ksy: /seq/1/cases/^AHEM:
 [info]         error: parsing expression '^AHEM' failed on 1:1, expected "not" ~ !(namePart) ~ not_test | comparison (SimpleMatchers.scala:34)
 [info] - switch_cases_malformed_quoting2 *** FAILED ***
@@ -534,7 +534,7 @@ tr
 [info]         error: missing mandatory argument `switch-on` (SimpleMatchers.scala:34)
 [info] - switch_on_malformed *** FAILED ***
 [info]   ..\tests\formats_err\switch_on_malformed.ksy: /seq/0:
-[info]         error: parsing expression '42/' failed on 1:3, expected "or" | CharsWhile(Set( , n)) | "\\\n" | End
+[info]         error: parsing expression '42/' failed on 1:3, expected Expected end-of-input:1:3, found "/"
 [info]    did not equal switch_on_malformed.ksy: /seq/0/switch-on:
 [info]         error: parsing expression '42/' failed on 1:3, expected "or" | CharsWhile(Set( , n)) | "\\\n" | End (SimpleMatchers.scala:34)
 [info] - top_empty *** FAILED ***

It's not that important, but we can think about how to improve this in the future.

addSbtPlugin("com.typesafe.sbt" % "sbt-native-packager" % "1.3.12")
addSbtPlugin("org.scala-js" % "sbt-scalajs" % "0.6.33")
addSbtPlugin("com.github.sbt" % "sbt-native-packager" % "1.9.16")
addSbtPlugin("org.scala-js" % "sbt-scalajs" % "1.13.1")
Copy link
Member

@generalmimon generalmimon Oct 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This causes a few changes in the generated js/npm/ folder after the sbt fastOptJS buildNpmJsFile buildNpmPackage invocation:

  1. The trick with providing an exportsNamespace that we later use to retrieve the MainJs object will no longer work, because the __ScalaJSEnv variable handling has been removed from the generated Scala.js code - see diff:

    @@ -9,24510 +9,37385 @@
     }(typeof self !== 'undefined' ? self : this, function () {
    
     var exports = {};
     var __ScalaJSEnv = { exportsNamespace: exports };
    
    +let MainJs;
     (function(){
     'use strict';
    -/* Scala.js runtime support
    - * Copyright 2013 LAMP/EPFL
    - * Author: Sébastien Doeraene
    - */
    -
    -/* ---------------------------------- *
    - * The top-level Scala.js environment *
    - * ---------------------------------- */
    -
    -
    -
    -
    -
    -// Get the environment info
    -var $env = (typeof __ScalaJSEnv === "object" && __ScalaJSEnv) ? __ScalaJSEnv : {};
    -
    -// Global scope
    -var $g =
    -  (typeof $env["global"] === "object" && $env["global"])
    -    ? $env["global"]
    -    : ((typeof global === "object" && global && global["Object"] === Object) ? global : this);
    -$env["global"] = $g;
    -
    -
    -
    -
    -// Where to send exports
    -
    -
    -
    -var $e =
    -  (typeof $env["exportsNamespace"] === "object" && $env["exportsNamespace"])
    -    ? $env["exportsNamespace"] : $g;
    -
    -$env["exportsNamespace"] = $e;
    @@ -97082,7 +108681,9 @@ var $d_scm_ArrayBuffer = new $TypeData().initClass({
       Ljava_io_Serializable: 1
     });
     $c_scm_ArrayBuffer.prototype.$classData = $d_scm_ArrayBuffer;
    -$e.MainJs = $m_Lio_kaitai_struct_MainJs$();
    +$L0 = new $c_RTLong(0, 0);
    +$d_J.zero = $L0;
    +MainJs = $m_Lio_kaitai_struct_MainJs$();
     }).call(this);
     //# sourceMappingURL=kaitai-struct-compiler-js-fastopt.js.map
    
    
     return exports.MainJs;
    
     }));

    (Notice our return exports.MainJs; statement at the end, which ends up evaluating to undefined, because the var exports = {}; variable hasn't changed since we initialized it.)

    So we need to adapt our building script:

    |}(typeof self !== 'undefined' ? self : this, function () {
    |
    |var exports = {};
    |var __ScalaJSEnv = { exportsNamespace: exports };
    |
    |$compiledFileContents
    |
    |return exports.MainJs;
    |
    |}));

  2. The $linkingInfo object properties have changed from "assumingES6": false to "assumingES6": true + "esVersion": 6, indicating that it's been compiled for ECMAScript 6. This is probably fine, ES6 comes from 2015 and thus it's very well supported in pretty much all modern browsers and environments (see https://compat-table.github.io/compat-table/es6/). Just wanted to point out this change, even though I don't think there's anything wrong with it.

    Nowadays, this change might only be a problem for special environments like Duktape, but I haven't seen any project that would be running the JS build of kaitai-struct-compiler via Duktape (the closest thing I'm aware of is https://github.com/taviso/kiewtai, but it uses Duktape only to run pre-generated JavaScript parsers and the KS runtime lib for JS). So that's really a niche concern, and there are tools like Babel that can transform ES6 code to be ES5 compatible if someone needs it.

     var $linkingInfo = {
    -  "envInfo": $env,
    -  "assumingES6": false,
    -  "semantics": {
    -    // ...
    -    "productionMode": false
    -  },
    -  "linkerVersion": "0.6.33",
    -  "globalThis": this
    +  "esVersion": 6,
    +  "assumingES6": true,
    +  "productionMode": false,
    +  "linkerVersion": "1.13.1",
    +  "fileLevelThis": this
     };

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generalmimon added a commit to Mingun/kaitai_struct_compiler that referenced this pull request Oct 23, 2023
Change commenting pattern so stupid VSCode folding algorithm correcly fold class
generalmimon added a commit to Mingun/kaitai_struct_compiler that referenced this pull request Oct 23, 2023
generalmimon added a commit to Mingun/kaitai_struct_compiler that referenced this pull request Oct 23, 2023
@@ -1,4 +1,4 @@
logLevel := Level.Warn

addSbtPlugin("com.typesafe.sbt" % "sbt-native-packager" % "1.3.12")
addSbtPlugin("com.github.sbt" % "sbt-native-packager" % "1.9.16")
Copy link
Member

@generalmimon generalmimon Oct 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: this update induces the following change (among others, which are just uninteresting refactorings) in the Bash script launcher in bin/kaitai-struct-compiler of generated artifacts:

--- 1/kaitai-struct-compiler-0.11-SNAPSHOT20231021.183147.746d773b/bin/kaitai-struct-compiler
+++ 2/kaitai-struct-compiler-0.11-SNAPSHOT20231023.110424.c6c6db7b/bin/kaitai-struct-compiler
   ...
@@ -267,7 +283,7 @@ loadConfigFile() {
 }

 # Now check to see if it's a good enough version
-# TODO - Check to see if we have a configured default java version, otherwise use 1.6
+# TODO - Check to see if we have a configured default java version, otherwise use 1.8
 java_version_check() {
   readonly java_version=$("$java_cmd" -version 2>&1 | awk -F '"' '/version/ {print $2}')
   if [[ "$java_version" == "" ]]; then
@@ -281,10 +297,10 @@ java_version_check() {
     if [[ "$major" -eq "1" ]]; then
      local major=$(echo "$java_version" | cut -d'.' -f2)
     fi
-    if [[ "$major" -lt "6" ]]; then
+    if [[ "$major" -lt "8" ]]; then
       echo
       echo The java installation you have is not up to date
-      echo $app_name requires at least version 1.6+, you have
+      echo $app_name requires at least version 1.8+, you have
       echo version $java_version
       echo
       echo Please go to http://www.java.com/getjava/ and download

So the version check has been updated to only accept Java 8+, as opposed to Java 6 as before. This change apparently comes from version https://github.com/sbt/sbt-native-packager/releases/tag/v1.9.12:

Merged pull requests:

At least in our case, it turned out that this only improves the error message, since the produced artifacts were compiled for at least Java 8 already.

If you try to run the Universal .zip build on Java 7, e.g. via Docker:

PS C:\temp\Mingun-upgrade-dependencies> docker run -it -v ${PWD}:/share azul/zulu-openjdk:7
root@699f1294788e:/# java -version
openjdk version "1.7.0_352"
OpenJDK Runtime Environment (Zulu 7.56.0.11-CA-linux64) (build 1.7.0_352-b01)
OpenJDK 64-Bit Server VM (Zulu 7.56.0.11-CA-linux64) (build 24.352-b01, mixed mode)
  1. If the universal .zip was built with a compiler version before this PR (https://github.com/kaitai-io/kaitai_struct_compiler/tree/746d773beb553c75f254a2a409a02fb4150b5b36), you'll get this:

    root@699f1294788e:/# /share/kaitai-struct-compiler-0.11-SNAPSHOT20231021.183147.746d773b/bin/kaitai-struct-compiler --version
    Exception in thread "main" java.lang.UnsupportedClassVersionError: io/kaitai/struct/JavaMain : Unsupported major.minor version 52.0
            at java.lang.ClassLoader.defineClass1(Native Method)
            at java.lang.ClassLoader.defineClass(ClassLoader.java:808)
            at java.security.SecureClassLoader.defineClass(SecureClassLoader.java:142)
            at java.net.URLClassLoader.defineClass(URLClassLoader.java:448)
            at java.net.URLClassLoader.access$100(URLClassLoader.java:65)
            at java.net.URLClassLoader$1.run(URLClassLoader.java:355)
            at java.net.URLClassLoader$1.run(URLClassLoader.java:349)
            at java.security.AccessController.doPrivileged(Native Method)
            at java.net.URLClassLoader.findClass(URLClassLoader.java:348)
            at java.lang.ClassLoader.loadClass(ClassLoader.java:430)
            at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:329)
            at java.lang.ClassLoader.loadClass(ClassLoader.java:363)
            at sun.launcher.LauncherHelper.checkAndLoadMain(LauncherHelper.java:588)
    
  2. If the universal .zip was built with a compiler version after this PR, it will produce a human-readable message:

    root@699f1294788e:/# /share/kaitai-struct-compiler-0.11-SNAPSHOT20231023.110424.c6c6db7b/bin/kaitai-struct-compiler --version
    
    The java installation you have is not up to date
    requires at least version 1.8+, you have
    version 1.7.0_352
    
    Please go to http://www.java.com/getjava/ and download
    a valid Java Runtime and install before running .
    

So, this is purely a slight improvement, not dropping existing support for anything.

Mingun and others added 8 commits October 23, 2023 19:26
Changes: sbt/sbt-native-packager@v1.3.12...v1.9.16

Organization name changed in the 1.9.0:
  com.typesafe.sbt -> com.github.sbt

Java 8 is required since 1.9.12
There dependencies depends on each other so updated simultaneously

scala-js 0.6.x no longer maintained: http://www.scala-js.org/doc/internals/scalajs-0.6.x-eol.html

scopt changelog: scopt/scopt@v3.6.0...v4.1.0

scala-js changelog: https://www.scala-js.org/news/index.html
- https://www.scala-js.org/news/2020/02/25/announcing-scalajs-1.0.0/
- https://www.scala-js.org/news/2020/03/10/announcing-scalajs-1.0.1/
- https://www.scala-js.org/news/2020/05/18/announcing-scalajs-1.1.0/
- https://www.scala-js.org/news/2020/07/02/announcing-scalajs-1.1.1/
- https://www.scala-js.org/news/2020/09/09/announcing-scalajs-1.2.0/
- https://www.scala-js.org/news/2020/10/16/announcing-scalajs-1.3.0/
- https://www.scala-js.org/news/2020/11/16/announcing-scalajs-1.3.1/
- https://www.scala-js.org/news/2021/01/12/announcing-scalajs-1.4.0/
- https://www.scala-js.org/news/2021/02/12/announcing-scalajs-1.5.0/ (Maven Central)
- https://www.scala-js.org/news/2021/04/01/announcing-scalajs-1.5.1/
- https://www.scala-js.org/news/2021/06/09/announcing-scalajs-1.6.0/ (Standard Scala Library 2.12.13)
- https://www.scala-js.org/news/2021/08/04/announcing-scalajs-1.7.0/ (Standard Scala Library 2.12.14)
- https://www.scala-js.org/news/2021/08/04/announcing-scalajs-1.7.1/ (Standard Scala Library 2.12.15)
- https://www.scala-js.org/news/2021/08/04/announcing-scalajs-1.8.0/ (Node.js 17)
- https://www.scala-js.org/news/2021/08/04/announcing-scalajs-1.9.0/ (strict floats)
- https://www.scala-js.org/news/2021/08/04/announcing-scalajs-1.10.0/ (address security vulnerability in java.util.UUID.randomUUID())
- https://www.scala-js.org/news/2021/08/04/announcing-scalajs-1.10.1/ (Standard Scala Library 2.12.16)
- https://www.scala-js.org/news/2022/09/15/announcing-scalajs-1.11.0/
- https://www.scala-js.org/news/2022/11/23/announcing-scalajs-1.12.0/ (Standard Scala Library 2.12.17)
- https://www.scala-js.org/news/2023/01/26/announcing-scalajs-1.13.0/ (Drop support Scala 2.11 and 2.12.1)
- https://www.scala-js.org/news/2023/04/10/announcing-scalajs-1.13.1/

scala-js 1.x prerequisites:
- Scala at least 2.11.12, 2.12.1 or 2.13.0                             OK, current compiler is 2.12.12
- Scala-js at least 0.6.33 to address all deprecation warnings         OK, upgrade from 0.6.33, no deprecation warnings
- Usage of sbt-crossproject                                            OK, upgraded in the previous commit
- Usage of `scalaJSLinkerConfig` instead of couple of other settings   OK, none of these settings are used
- sbt 1.2.1 or later                                                   OK, current sbt is 1.7.1

scala-js artifacts hosted on the Maven Central since 1.5.0
Note that we're not reducing security by no longer explicitly using
`SafeConstructor`, as this is the default since SnakeYAML 2.0. See the
references below:

1. https://snyk.io/blog/snakeyaml-unsafe-deserialization-vulnerability/

   > SnakeYaml 2.0 was released in early 2023 to mitigate the default
   > behavior that can lead to possible arbitrary code execution. In
   > this version, the constructor that every new yaml() uses now
   > extends SafeConstructor.

2. https://www.veracode.com/blog/research/resolving-cve-2022-1471-snakeyaml-20-release-0

   > Since 2.0, the SnakeYAML `Constructor` now inherits from
   > `SafeConstructor`, which prevents an attacker from accessing the
   > classpath by default.
Copy link
Member

@generalmimon generalmimon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Mingun Thanks a lot, merging in!

@generalmimon generalmimon merged commit 9247f44 into kaitai-io:master Oct 23, 2023
@Mingun Mingun deleted the upgrade-dependencies branch October 23, 2023 18:48
@Mingun
Copy link
Contributor Author

Mingun commented Oct 23, 2023

Thanks you that you finally get a time to review and finish this. Glad to see any movements in the project!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sbt: cross project
2 participants