Update the Gazelle Java plugin to handle java_maven_install_file directives in nested directories#384
Conversation
|
@shs96c - Does this look like a reasonable bug fix to bring in? |
|
Apologies for the slow review. Looking now. |
shs96c
left a comment
There was a problem hiding this comment.
I've a feeling that this isn't going to work when the lock file isn't in the repo root. That's because MavenInfoFile() also looks for the lock file against the repo root and not the directory where the directive was discovered. I think you'll need to store the full path to the lock file and not just the path given by the directive.
There was a problem hiding this comment.
How come this file is the v1 lock file format and not the v2 format?
There was a problem hiding this comment.
This should also be a v2 lock file
There was a problem hiding this comment.
I updated one of the lock files to v2 to show that it doesn't affect the behavior. I can update the other lock file but it isn't critical since the plugin should continue to support v1 lock files.
What do you think of an update to run //:pin when UPDATE_SNAPSHOTS=true is set? That would make it easier to update the lock files as needed.
… a global one for all directories
… and that it works with V2 of the file.
d2de978 to
bbbd107
Compare
I've updated the test case to show that providing the full path to the lock file will work. I believe this matches the current behavior because there aren't any changes to |
Create a
maven.Resolverfor each instance ofjava_maven_install_fileand use that resolver for all files under that directory. There aren't any changes to the default behavior; but nestedjava_maven_install_filedeclarations will now work as expected.Background
The Gazelle plugin parses
java_maven_install_filedirectives in nested directories but doesn't use the file when resolving dependencies. This is a problem for projects that use different Maven repositories for different parts of the repository. The top-levelmaven_install.jsonfile may have incomplete or extra dependencies than the one configured for the lower level directory.Testing
The unit tests have been updated to reflect the new interface. Integration tests have been added to confirm the updated behavior for an invalid
maven_install.jsonfile.