m2local: solve files not getting resolved #961
Conversation
gergelyfabian
requested review from
jin,
Wyverald,
shs96c and
cheister
as code owners
|
This is demonstration PR first and foremost (how the reported problem could be fixed - I use this branch for local testing). |
|
Probably related to #935. |
|
To be exact, yes, I use m2local, and I pin the artifacts. |
There was a problem hiding this comment.
Hi @gergelyfabian. Thank you for prepping the PR and explaining how and why it works, and my apologies for a slow turn-around on this review. That's really appreciated.
Could you please add a test for this, as it's the kind of thing that I think will easily slip through the cracks as we make changes? You may find it easiest to add something to the tests/bazel_run_tests.sh script we have: there's definitely at least one test in there to do with working with m2local. If you need a hand with this, please comment here or just ping me on the public Bazel slack and I'll do what I can.
| @@ -189,4 +193,5 @@ v2_lock_file = struct( | |||
| get_artifacts = _get_artifacts, | |||
| get_netrc_entries = _get_netrc_entries, | |||
| render_lock_file = _render_lock_file, | |||
| has_m2local = _has_m2local, | |||
There was a problem hiding this comment.
You'll need to make a matching change in the v1_lock_file.bzl file too
Thank you! Great advice. I tried and realized that the aspect I was fixing in this PR is not covered in those tests (quite expected though). Thinking about current master: I'll try covering this (and more) with tests. |
|
What's more, even just this will fail: |
|
Will update the PR, two separate commits: 1. fix, 2. test extensions. One could prove the tests are reproducing this issue by reverting the fix (and then they fail). |
|
Here is the reproduction from the tests: |
gergelyfabian
force-pushed
the
load_m2local_artifacts_properly
branch
from
7069798
to
457849f
Compare
|
Missing parts added, tests added (reverting the fix fails them), windows issues solved. All tests are passing. Should be ready for re-review. |
|
@shs96c did you have a chance to review this one maybe? I think all issues should be solved. |
Coursier can resolve m2local artifacts when pinning. However, if you try using them, they will fail stamping: ``` Stamping the manifest of @maven//:dev_zio_zio_test_2_12 failed: missing input file 'external/maven/dev/zio/zio-test_2.12/2.0.18-RC0-whatever/zio-test_2.12-2.0.18-RC0-whatever.jar', owner: '@maven//:dev/zio/zio-test_2.12/2.0.18-RC0-whatever/zio-test_2.12-2.0.18-RC0-whatever.jar' ``` This is assuming, that I have built dev.zio:zio-test-junit_2.12:2.0.18-RC0-whatever locally and deployed it to ~/.m2/repository. There are two reasons to that: 1. In coursier.bzl we are not adding a local url to the generated http_file() usage (so it does not download it) 2. A corresponding genrule entry is not generated in dependency_tree_parser.bzl Fix it by adding a local url in coursier.bzl and ensure a genrule is added. This way the file is downloaded and then properly copied (and is available for stamping). Added has_m2local to v1_lock_file.bzl and v2_lock_file.bzl.
Extended pinning test cases with also building the local artifact (the bug is only reproducible with pinning and occurs when building the artifact).
gergelyfabian
force-pushed
the
load_m2local_artifacts_properly
branch
from
f67e912
to
a9a3c1d
Compare
Coursier can resolve m2local artifacts when pinning. However, if you try using them, they will fail stamping:
This is assuming, that I have built dev.zio:zio-test-junit_2.12:2.0.18-RC0-whatever locally, deployed it to ~/.m2/repository and then set up a dependency on it, enabled m2local and tried using the artifact.
There are two reasons to that:
Fix it by adding a local url in coursier.bzl and ensure a genrule is added. This way the file is downloaded and then properly copied (and is available for stamping).
Added has_m2local to v2_lock_file.bzl.