-
Notifications
You must be signed in to change notification settings - Fork 31
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
Transitive dependencies are conflicting #52
Comments
I tried to gain a better understanding of what's going on here, and it seems to me that the conflict for the issue n.2 arises because all the transitive dependencies are flatted up to the module level. So if my project, say
It flattens dependencies this way (correct me if I'm wrong):
This happens during the
You can see it looses the nesting, while the correct format should be: I was figuring out how to make this, but it seems to me that transitive dependencies are flatted up during the Do you have any idea? +friendly ping @pcj |
Sorry @lexor90 this has not recvd the attention it deserves! I hope to look at this in more detail but don't have a time estimate. |
Hey there,
After upgrading to newer versions of bazel I started using this rules (previously I was using an in-house version that had similiar concepts to this library before the latest major rewrite - no package.json support -).
Today I added these rules and started rewriting the repository to use these ones, and soon I faced two issues:
was already ticketed in Can't use mocha_test on non-root packages #37. Specifically, we have tests in a each project's subdirectory "tests/". Specifying the
main = "tests/index-test.js"
does not work because of the way themocha_tests
rule declares the entrypoint (it uses the rulename
). I've seen you patched it using PACKAGE_NAME, but, correct me if I'm wrong this would require aBUILD
file inside of thetests/
directory - side note: furthermore PACKAGE_NAME is now deprecated in favor ofpackage_name()
-. It was not comfortable for me to add several thousand of new build files, so I simply patched themocha_test
rule by adding:if len(main.split("/")) > 1:
for directory in main.split("/")[:-1]:
entrypoint.append(directory)
This way let's assume the rule is:
It now properly appends to
entrypoint
thetests/
path (previously it was referring to thetest_module
root path, resulting in mocha unable to find tests) finding the test files correctly.Maybe there are better ways to do this, but I didn't want to rewrite the full rule (moreover I'm still getting used to the new APIs coming with Bazel >= 6). This has been tested with bazel 6 and bazel 8.
I don't know if this also affects previous versions, but since the creation of the
node_modules()
rules, if you declare multiple packages (mixingnode_module
andyarn_modules
) that have some npm/yarn package in common (but with different versions) bazel stops because offile 'output/project/test_modules/copy_library.sh' is generated by these conflicting actions
. I understand that this is one of Bazel's principles (to avoid diamond dependencies) and while I perfectly agree in the open-source world this is very difficult to achieve.I tracked it down and applied a quick fix, in order to continue the integration, but I'm pretty unsure about the best thing to do here. Right now I'm filtering transitive dependencies in
node_modules.bzl
's_copy_modules()
by their identifier. This is the modified version:This way we do only produce a single
copy_library.sh
per identifier. But this also comes with problems: different versions. Each yarn module uses its own version of a library, and while a minor upgrade might be pretty safe to assume to be working, a major update would be scaring. In my case, several packages rely on thehoek
package with versions ranging from 2.x to 4.x. And my patch currently copies only one of them (which one is undefined). While tests might get me covered I don't think this is a proper solution. I was thinking to rename the "copy_X.sh" to something including the version name, but this would not fix the final result (node_modules will contain "hoek"), while adding the version also to the library itself does break therequire
lookup -of course-).What are your thoughts about it? Maybe applying some concept such as nested
node_modules
could work, but I don't know if there are other downsides to this approach. Right now it seems to me to be the best solution.P.S. feel free to rename this issue, I could not think a better title. Also should you prefer I could split the two issues into two separated tickets.
The text was updated successfully, but these errors were encountered: