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

Fixing scalap version for Scala 2.11.12 #1206

Merged
merged 1 commit into from
Feb 9, 2021

Conversation

gergelyfabian
Copy link
Contributor

Description

The hash and version for scalap for Scala 2.11.12 conflicted.
Set up both version and hash for scalap to be for Scala 2.11.12 (as other Scala components are set for Scala 2.11).

Motivation

The sha256 is set for scalap:2.11.12, but the version set was 2.11.10 which
caused a hash conflict.

$ sha256sum scalap-2.11.12.jar
a6dd7203ce4af9d6185023d5dba9993eb8e80584ff4b1f6dec574a2aba4cd2b7 scalap-2.11.12.jar
$ sha256sum scalap-2.11.10.jar
3f6f352fce91c33055398a7081e35e5091fd5e095130905633e72f52124c1d27 scalap-2.11.10.jar

The sha256 is set for scalap:2.11.12, but the version set was 2.11.10 which
caused a hash conflict.

$ sha256sum scalap-2.11.12.jar
a6dd7203ce4af9d6185023d5dba9993eb8e80584ff4b1f6dec574a2aba4cd2b7  scalap-2.11.12.jar
$ sha256sum scalap-2.11.10.jar
3f6f352fce91c33055398a7081e35e5091fd5e095130905633e72f52124c1d27  scalap-2.11.10.jar
@gergelyfabian
Copy link
Contributor Author

This is seems to be a regression in commit: d8f5bd9.

-            "org_scala_lang_scalap": {
-                "version": "2.11.12",
-                "sha256": "a6dd7203ce4af9d6185023d5dba9993eb8e80584ff4b1f6dec574a2aba4cd2b7",
-            },
+    "org_scala_lang_scalap": {
+        "artifact": "org.scala-lang:scalap:2.11.10",
+        "sha256": "a6dd7203ce4af9d6185023d5dba9993eb8e80584ff4b1f6dec574a2aba4cd2b7",
+        "deps": [
+            "@io_bazel_rules_scala_scala_compiler",
+        ],
+    },

@liucijus
Copy link
Collaborator

liucijus commented Feb 8, 2021

@gergelyfabian thank you!

Copy link
Member

@ittaiz ittaiz left a comment

Choose a reason for hiding this comment

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

Thanks!

@ittaiz ittaiz merged commit 22a67e1 into bazelbuild:master Feb 9, 2021
@ittaiz
Copy link
Member

ittaiz commented Feb 9, 2021

@liucijus do you think we should add a CI run that is without any caches? That would have caught this issue

@liucijus
Copy link
Collaborator

liucijus commented Feb 9, 2021

@liucijus do you think we should add a CI run that is without any caches? That would have caught this issue

I guess we do not run full test suite for all versions (this is 2.11 ). Not sure if we should do that though - it's expensive CI run and is relatively easy to fix on the user side.

@gergelyfabian
Copy link
Contributor Author

gergelyfabian commented Feb 9, 2021

@liucijus do you think we should add a CI run that is without any caches? That would have caught this issue

I guess we do not run full test suite for all versions (this is 2.11 ). Not sure if we should do that though - it's expensive CI run and is relatively easy to fix on the user side.

I do agree that this kind of test is very expensive. Getting all jars, to propagate the repository cache... That's kind of tough :)

@ittaiz
Copy link
Member

ittaiz commented Feb 9, 2021

Would we have caught it for 2.12?

@gergelyfabian
Copy link
Contributor Author

Would we have caught it for 2.12?

I don't think so, because this was that kind of regression that you retrieved the proper file with the proper hash once (before the regression), then you change the file's url, leaving the same hash, so Bazel won't try to refetch it. It will only be found on a clean repository cache (exactly this happened in our project, this bug was found only when trying to build on clean environments).

@gergelyfabian gergelyfabian deleted the scala2.11_scalap_version branch February 9, 2021 07:21
@ittaiz
Copy link
Member

ittaiz commented Feb 9, 2021

I think it's worth it to run such a clean test on 2.12 only but it's not that important to me

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

Successfully merging this pull request may close these issues.

3 participants