Skip to content

Upgrade to Bazel 0.12.0 #481

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

Conversation

pauldraper
Copy link

No description provided.

@pauldraper
Copy link
Author

Failing on test_scala_library_expect_failure_on_missing_direct_java. Looks like we're losing the label.

@ittaiz
Copy link
Contributor

ittaiz commented Apr 13, 2018

This is strange. @tomlu @cushon any chance you can take a look? I think this is related to the SJD changes though the odd thing here is that AFAICT we are creating the ijar here when we compile java files. https://github.com/bazelbuild/rules_scala/blob/master/scala/scala.bzl#L321

@cushon
Copy link
Contributor

cushon commented Apr 13, 2018

ijars created by java_common.compile do not currently get stamped, and we can't fix that until the label is passed in to the API. This is partially what is being discussed in https://groups.google.com/forum/#!topic/bazel-discuss/mt2llfwzmac.

@ittaiz
Copy link
Contributor

ittaiz commented Apr 14, 2018 via email

@johnynek
Copy link
Contributor

looks like the test for strict deps needs to be updated. It failed.

@ittaiz
Copy link
Contributor

ittaiz commented Apr 24, 2018 via email

@ittaiz
Copy link
Contributor

ittaiz commented Apr 29, 2018

ok, the situation here is a bit complicated.
Bazel 0.12.0 indeed broke strict-deps add_dep support for code that doesn't use ijar (for example if you use the current scala_import or the filegroup from maven_jar)
internally we've worked around it by having a script iterate the log and take various hints to fix the suggestions but this is heavily based on internal assumptions.
The solution for most issues (except for exports) will probably come from: https://docs.google.com/document/d/1ubah6phuvWnugShtVgSQnaopQ1BtKtNxQASVwGZA7k0/edit?usp=sharing

I think the pragamatic approach here is to loosen the assertion in the test so that it covers strict-deps error but not the add_deps message until the above is implemented and supported in rules_scala.
This will allow to move forward with rules_scala's CI versions.

WDYT?

cc @natansil @or-shachar for thoughts as well

@natansil
Copy link
Contributor

@ittaiz I'm fine with loosening the tests. it's just the java ones right?
I suppose an issue should be opened that states the add_deps message tests should be re-instated once the solution is implemented in bazel itself.

@johnynek
Copy link
Contributor

Note, we are using the rules with bazel 0.12 at Stripe. No issues that I know of yet.

@jjudd
Copy link
Contributor

jjudd commented Apr 29, 2018

Lucid has also been running on 0.12 without issue. Strict/unused dep recommendations did break for us, so we changed all of our java_imports to scala_imports and are running off the work done in this branch: #487

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.

7 participants