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

Use version-aware naming for artifact repositories #1573

Merged
merged 3 commits into from
May 13, 2024

Conversation

aszady
Copy link
Contributor

@aszady aszady commented May 8, 2024

Description

It enables the logic implemented in #1562. The choice of toolchains to cover is the same as in #1566.

Now, e.g., instead of referring to the artifact's repository @io_bazel_rules_scala_scala_compiler, we need to may use the version-aware name, like @io_bazel_rules_scala_scala_compiler_3_3_0.

In future this may require an update in client's code. In particular in cases they don't use the default toolchains provided by the rules, and construct their own dependency providers. One would need to manually add the version suffix to the repository names, or use a construction like + version_suffix(SCALA_VERSION) in their code.

For now a backward compatibility layer is provided in a shape of repository with aliases, pointing to artifacts for the default Scala version.

Motivation

#1290

@aszady aszady marked this pull request as ready for review May 8, 2024 14:12
Copy link
Collaborator

@simuons simuons left a comment

Choose a reason for hiding this comment

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

Thanks, @aszady,

Could you please add aliases for backwards compatibility as you proposed

providing some kind of an alias that points from non-suffixed repository to the one with default Scala version

Not sure how complicated that would be, but definitely better than adding ifs on default scala version all over the place. Later we could deprecate those aliases and allow people to migrate.

@aszady aszady force-pushed the artifacts branch 2 times, most recently from ea7b262 to 35a6ed8 Compare May 9, 2024 14:22
@aszady
Copy link
Contributor Author

aszady commented May 9, 2024

Added. Out of possible solutions, two aliases worked the best for me.
I also tried sth like using scala_library with single dependency, though it didn't work well with dependency checker (suggesting unused dependency).
@simuons, please take a look at the revised version.

Now this PR consists of three commits:

  1. Preparation: add alias repo for backward compatibility.
  2. Do the actual change (this state demonstrates that aliases work).
  3. Cleanup: migrate all the internal rules' deps to suffixed repos.

Do you prefer to have them merged together or separately?

Copy link
Collaborator

@simuons simuons left a comment

Choose a reason for hiding this comment

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

Thanks, @aszady

@simuons simuons merged commit 6b19a31 into bazelbuild:master May 13, 2024
2 checks passed
@aszady aszady deleted the artifacts branch June 11, 2024 17:52
mbland added a commit to EngFlow/example that referenced this pull request Sep 9, 2024
rules_scala v6.6.0 updated all its repo names to include the version
number in the suffix.

- https://github.com/bazelbuild/rules_scala/releases/tag/v6.6.0
- bazelbuild/rules_scala: Use version-aware naming for artifact
  repositories #1573
  bazelbuild/rules_scala#1573

Hence, now all the repos and toolchains are appended with the scala
version suffix.

My next PR will update the module extensions to enable setting our own
Scala version. The intention is to eventually try to contribute these
extensions (and other necessary changes) upstream.

Signed-off-by: Mike Bland <mbland@engflow.com>
mbland added a commit to EngFlow/example that referenced this pull request Sep 9, 2024
rules_scala v6.6.0 updated all its repo names to include the version
number in the suffix.

- https://github.com/bazelbuild/rules_scala/releases/tag/v6.6.0
- bazelbuild/rules_scala: Use version-aware naming for artifact
  repositories
  bazelbuild/rules_scala#1573

Hence, now all the repos and toolchains are appended with the scala
version suffix.

My next PR will update the module extensions to enable setting our own
Scala version. The intention is to eventually try to contribute these
extensions (and other necessary changes) upstream.

---------

Also, I had to pull rules_java back to 7.9.1 because 7.10.0 has problems 
on non-macOS platforms:

- bazelbuild/rules_java: Regression with
  @@rules_java//toolchains:bootstrap_runtime_toolchain_type in 7.10.0
  bazelbuild/rules_java#214

- bazelbuild/rules_java: Rules java upgrade to 7.10.0 causes issues on
  Windows 
  bazelbuild/rules_java#218

The latter issue seems to match what we're seeing in CI: 

- https://github.com/EngFlow/example/actions/runs/10778667925/job/29890522476?pr=339

```txt
ERROR: no such package '@@rules_java~//tools/jdk': BUILD file not found
in directory 'tools/jdk' of external repository @@rules_java~. Add a
BUILD file to a directory to mark it as a package.

ERROR:
OUTPUT_BASE/external/rules_java~~toolchains~local_jdk/BUILD.bazel:18:10:
no such package '@@rules_java~//tools/jdk': BUILD file not found in
directory 'tools/jdk' of external repository @@rules_java~. Add a BUILD
file to a directory to mark it as a package. and referenced by
'@@rules_java~~toolchains~local_jdk//:bootstrap_runtime_toolchain_definition'
```

---------

Signed-off-by: Mike Bland <mbland@engflow.com>
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.

3 participants