-
Notifications
You must be signed in to change notification settings - Fork 277
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
Automated creating and updating scala_x_x.bzl repository files #1608
Automated creating and updating scala_x_x.bzl repository files #1608
Conversation
c703f8e
to
3248e01
Compare
scala_major = ".".join(scala_version.split(".")[:2]) | ||
scala_test_major = "3" if scala_major >= "3.0" else scala_major | ||
scala_fmt_major = "2.13" if scala_major >= "3.0" else scala_major | ||
kind_projector_version = "0.13.2" if scala_major < "2.13" else "0.13.3" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some versions are not compatible/doesn't exist in certain Scala version
|
||
def get_maven_coordinates(artifact) -> MavenCoordinates: | ||
splitted = artifact.split(':') | ||
version = splitted[2] if splitted[2][0].isnumeric() else splitted[3] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The version location in maven coordinates isn't strict, so there are cases that the version is under the x+1 index after splitting it by ':'
data = json.load(file) | ||
return get_mavens_coordinates_from_json(dependency["directDependencies"]) if any((dependency := d)["coord"] == artifact for d in data["dependencies"]) else [] | ||
|
||
def get_label(coordinate) -> str: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are 5 patterns of labels, which are related to the structure of certain group and artifact due to custom naming.
temp = {} | ||
|
||
for a in artifacts: | ||
label = get_label(a.coordinates).replace('scala3_', 'scala_').replace('scala_tasty_core', 'scala_scala_tasty_core') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's related to custom naming convention of certain labels, TBD in the future to simplify it.
def is_that_trailing_coma(content, char, indice) -> bool: | ||
return content[indice] == char and content[indice+1] != ',' and content[indice+1] != ':' and content[indice+1] != '@' and not content[indice+1].isalnum() | ||
|
||
def get_with_trailing_commas(content) -> str: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After mapping to the dict, there are no trailing commas, so this is a workaround to adjust file to .bzl files / starlark convention and CI-checks failures.
path = os.getcwd().replace('/scripts', '/third_party/repositories') | ||
file = Path(f'{path}/{'scala_' + "_".join(version.split(".")[:2]) + '.bzl'}') | ||
|
||
if not file.exists(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If file doesn't exist, the script is copying the content from last file in directory (default sort) and updating it. So it won't work when someone is creating e.g. scala_2_10.bzl (since the heuristic is copying the content from scala_3_5.bzl in this example).
data.write(line) | ||
|
||
with file.open('r+') as data: | ||
excluded_artifacts = ["org.scala-lang.modules:scala-parser-combinators_2.11:1.0.4"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are artifacts versions that cause CI-check failures after update and exceptions in logs.
original_artifact_dict[label]["artifact"] = artifact | ||
original_artifact_dict[label]["sha256"] = sha | ||
if deps: | ||
dependencies = [d for d in deps if d[1:] in labels and "runtime" not in d and "runtime" not in artifact] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not modifying the runtime dependencies due to CI-checks failure.
|
||
def map_to_resolved_artifacts(output) -> List[ResolvedArtifact]: | ||
resolved_artifacts = [] | ||
subprocess.call(f'cs fetch {' '.join(output)} --json-output-file out.json', shell=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Creates local file to get the directDependencies for fetched artifacts
@@ -2,68 +2,144 @@ scala_version = "2.11.12" | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now scala_version variable is needed due to repositories.bzl
file and other usages, so it can be changed in the future.
@liucijus @simuons Could you take a look at this PR? It is a first step to improve maven dependency management in rules_scala. It will allow to bump patch versions of scala and add new minor versions faster. Eventually we would like to organize dependencies more, to keep naming consistent, have clear understanding which deps belong into which resolution tree, etc. and have more frequent updates of other dependencies too. |
@liucijus @simuons Ping for taking a look at this PR. It adds Scala 3.5.0, while at the same time, we're in the middle of the 3.5.2 release and its artifacts are already available, public release by the end of this week. scala/scala3#21781 As a Scala 3 compiler team, we'd like to help maintain the rules and keep it up to date. The release plan for Scala 3 involves a new stable release every 6-8 weeks. We'd like to include ensuring the new version of compiler would be supported by Bazel scala_rules as part of our release procedure in the similar way as we're dealing currently with other vital parts of Scala tooling (eg. Debug Adapter Protocol, Metals, Scala CLI, Scastie) Having a tool to easily add new version would be really beneficial here, but we'd also require a bit of involvement from the maintainers of Scala rules to ensure changes would be reviewed and merged within a finite time span. I belive that if needed both me and @lukaszwawrzyk would consider getting the maintainer role in this project. If needed we can arrange some discussions on how @VirtusLab, as organization, can help with the development of this project. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
17d3836
to
6f46032
Compare
@simuons Done :) I thought that these changes caused CI failure: |
@lm1nrt, I'm looking why build is failing. This is not related to your changes. Failing task was supposed to be optional. |
Hi, @lm1nrt, I'm really sorry but I'll ask you to rebase once again. Master is green now, so your PR should be as well. |
d12d14c
to
5e49f53
Compare
Description
It's not the ideal version, but I believe it can be improved by adding support for more Scala artifacts.
Motivation
Updating and creating new versions for scala_x_x.bzl files is time-consuming, and it's a kind of work that is not performed regularly due to its specifics.