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.

Sign up for GitHub

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

Jump to bottom

Regression in 1.5.0 generated rules #1787

Closed
purkhusid opened this issue Apr 4, 2020 · 2 comments · Fixed by #1816
Closed

Regression in 1.5.0 generated rules #1787

purkhusid opened this issue Apr 4, 2020 · 2 comments · Fixed by #1816

Comments

@purkhusid
Copy link
Contributor

🐞 bug report

Affected Rule

The issue is related to the auto generated rules for node_modules/.bin

Is this a regression?

Yes, this works in 1.4.1

Description

I'm using the auto genrated rules to run GraphQL Code Generator(https://graphql-code-generator.com)

It works fine in version 1.4.1 but in 1.5.0 in fails.
I've created a minimal reproduction of the issue here: https://github.com/purkhusid/glql-gen-repro

🔥 Exception or Error


Cannot use GraphQLEnumType "__TypeKind" from another module or realm.

Ensure that there is only one instance of "graphql" in the node_modules
directory. If different versions of "graphql" are the dependencies of other
relied on modules, use "resolutions" to ensure only one version is installed.

https://yarnpkg.com/en/docs/selective-version-resolutions

Duplicate "graphql" modules cannot be used at the same time since different
versions may have different capabilities and behavior. The data from one
version used in the function from another could produce confusing and
spurious results.
[21:03:30] Generate types.ts [failed]
[21:03:30] → Cannot use GraphQLEnumType "__TypeKind" from another module or realm.

Ensure that there is only one instance of "graphql" in the node_modules
directory. If different versions of "graphql" are the dependencies of other
relied on modules, use "resolutions" to ensure only one version is installed.

https://yarnpkg.com/en/docs/selective-version-resolutions

Duplicate "graphql" modules cannot be used at the same time since different
versions may have different capabilities and behavior. The data from one
version used in the function from another could produce confusing and
spurious results.

🌍 Your Environment

Operating System:

  
MacOS 1.14

Output of bazel version:

  
Build label: 2.2.0
Build target: bazel-out/darwin-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
Build time: Tue Mar 3 09:28:15 2020 (1583227695)
Build timestamp: 1583227695
Build timestamp as int: 1583227695

Rules_nodejs version:

  
    1.5.0
@alexeagle
Copy link
Collaborator

Thanks for the small repro.

Regression must be in these commits: 1.4.1...1.5.0
I confirmed it's still present at HEAD.
I tried a diff of bazel aquery :gen in both the working and broken state, thinking that maybe the action is being passed different inputs, but the diff was only the action key SHA

Still looking for the cause.

@alexeagle
Copy link
Collaborator

I bisected it manually, it's caused by a6e29c2

alexeagle added a commit to alexeagle/rules_nodejs that referenced this issue Apr 10, 2020
@alexeagle
fix(builtin): look in the execroot for nodejs_binary source entry_points
9131c9e 
In a6e29c2 we added support for generated entry_point by adding a secondary lookup.
In 863c7de we reversed the order of the lookups to make rollup work in a certain use case.
Neither of these was principled, because we know ahead of time whether the entry_point is generated. We can use a single lookup.

This also makes sure that programs run in the location where the linker will put them (note that the logic in question runs before the linker has created the node_modules directory in the execroot.)
This is why bazel-contrib#1787 observes that some node programs were broken - we were running the entry point as
bazel-out/host/bin/external/npm/@graphql-codegen/cli/bin/graphql-codegen.sh.runfiles/npm/node_modules/@graphql-codegen/cli/bin.js
when it should have been
node_modules/@graphql-codegen/cli/bin.js

Fixes bazel-contrib#1787
@alexeagle alexeagle mentioned this issue Apr 10, 2020
Merged
alexeagle added a commit to alexeagle/rules_nodejs that referenced this issue Apr 10, 2020
@alexeagle
fix(builtin): look in the execroot for nodejs_binary source entry_points
244eb24 
In a6e29c2 we added support for generated entry_point by adding a secondary lookup.
In 863c7de we reversed the order of the lookups to make rollup work in a certain use case.
Neither of these was principled, because we know ahead of time whether the entry_point is generated. We can use a single lookup.

This also makes sure that programs run in the location where the linker will put them (note that the logic in question runs before the linker has created the node_modules directory in the execroot.)
This is why bazel-contrib#1787 observes that some node programs were broken - we were running the entry point as
bazel-out/host/bin/external/npm/@graphql-codegen/cli/bin/graphql-codegen.sh.runfiles/npm/node_modules/@graphql-codegen/cli/bin.js
when it should have been
node_modules/@graphql-codegen/cli/bin.js

Fixes bazel-contrib#1787
alexeagle added a commit to alexeagle/rules_nodejs that referenced this issue Apr 10, 2020
@alexeagle
fix(builtin): look in the execroot for nodejs_binary source entry_points
f81e946 
In a6e29c2 we added support for generated entry_point by adding a secondary lookup.
In 863c7de we reversed the order of the lookups to make rollup work in a certain use case.
Neither of these was principled, because we know ahead of time whether the entry_point is generated. We can use a single lookup.

This also makes sure that programs run in the location where the linker will put them (note that the logic in question runs before the linker has created the node_modules directory in the execroot.)
This is why bazel-contrib#1787 observes that some node programs were broken - we were running the entry point as
bazel-out/host/bin/external/npm/@graphql-codegen/cli/bin/graphql-codegen.sh.runfiles/npm/node_modules/@graphql-codegen/cli/bin.js
when it should have been
node_modules/@graphql-codegen/cli/bin.js

Fixes bazel-contrib#1787
gregmagolan pushed a commit to alexeagle/rules_nodejs that referenced this issue Apr 11, 2020
@alexeagle @gregmagolan
fix(builtin): look in the execroot for nodejs_binary source entry_points
7955dbc 
In a6e29c2 we added support for generated entry_point by adding a secondary lookup.
In 863c7de we reversed the order of the lookups to make rollup work in a certain use case.
Neither of these was principled, because we know ahead of time whether the entry_point is generated. We can use a single lookup.

This also makes sure that programs run in the location where the linker will put them (note that the logic in question runs before the linker has created the node_modules directory in the execroot.)
This is why bazel-contrib#1787 observes that some node programs were broken - we were running the entry point as
bazel-out/host/bin/external/npm/@graphql-codegen/cli/bin/graphql-codegen.sh.runfiles/npm/node_modules/@graphql-codegen/cli/bin.js
when it should have been
node_modules/@graphql-codegen/cli/bin.js

Fixes bazel-contrib#1787
alexeagle added a commit to alexeagle/rules_nodejs that referenced this issue Apr 11, 2020
@alexeagle
fix(builtin): look in the execroot for nodejs_binary source entry_points
3d6e003 
In a6e29c2 we added support for generated entry_point by adding a secondary lookup.
In 863c7de we reversed the order of the lookups to make rollup work in a certain use case.
Neither of these was principled, because we know ahead of time whether the entry_point is generated. We can use a single lookup.
However that depends on running the linker on all generated bin rules, which is a breaking change, so for now we just defer the FS check until after the linker runs.

This also makes sure that programs run in the location where the linker will put them (note that the logic in question runs before the linker has created the node_modules directory in the execroot.)
This is why bazel-contrib#1787 observes that some node programs were broken - we were running the entry point as
bazel-out/host/bin/external/npm/@graphql-codegen/cli/bin/graphql-codegen.sh.runfiles/npm/node_modules/@graphql-codegen/cli/bin.js
when it should have been
node_modules/@graphql-codegen/cli/bin.js

Fixes bazel-contrib#1787
gregmagolan pushed a commit that referenced this issue Apr 11, 2020
@alexeagle
fix(builtin): look in the execroot for nodejs_binary source entry_poi…
b84d65e 
…nts (#1816)

In a6e29c2 we added support for generated entry_point by adding a secondary lookup.
In 863c7de we reversed the order of the lookups to make rollup work in a certain use case.
Neither of these was principled, because we know ahead of time whether the entry_point is generated. We can use a single lookup.
However that depends on running the linker on all generated bin rules, which is a breaking change, so for now we just defer the FS check until after the linker runs.

This also makes sure that programs run in the location where the linker will put them (note that the logic in question runs before the linker has created the node_modules directory in the execroot.)
This is why #1787 observes that some node programs were broken - we were running the entry point as
bazel-out/host/bin/external/npm/@graphql-codegen/cli/bin/graphql-codegen.sh.runfiles/npm/node_modules/@graphql-codegen/cli/bin.js
when it should have been
node_modules/@graphql-codegen/cli/bin.js

Fixes #1787
@ptsteadman ptsteadman mentioned this issue Jan 19, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(builtin): look in the execroot for nodejs_binary source entry_points alexeagle/rules_nodejs
2 participants
@alexeagle @purkhusid