do not index py_binary unless in file mode by linzhp · Pull Request #2989 · bazel-contrib/rules_python

linzhp · 2025-06-15T04:29:16Z

This is yet another implementation of #2822. Based on this:

Lines 248 to 252 in 94e08f7

// Remove the file from srcs if we're doing per-file library generation so
// that we don't also generate a py_library target for it.
ifcfg.PerFileGeneration() { 
srcs.Remove(name) 
 } 

The only case when a python file does not exist in py_library is in file mode. Stop indexing py_binary in other mode to avoid ambiguous dependency resolution.

linzhp · 2025-06-23T04:52:03Z

This PR is ready for review. The failing CI is an infra issue

linzhp · 2025-06-29T15:05:03Z

Added tests. Please take another look

linzhp · 2025-07-13T17:43:06Z

@aignas friendly ping...

dougthor42

I tested this on our big repo that uses file mode; looks good (eg gazelle didn't cause any unexpected changes).

Request: please update CHANGELOG.md with a fixed entry.

For my own future reference, this is what happens without the fix (eg reverting e9bd803):

=================== Test output for //python:python_test_import_with_same_srcs_library_and_binary:
--- FAIL: TestGazelleBinary (0.00s)
--- FAIL: TestGazelleBinary/import_with_same_srcs_library_and_binary (0.03s)
python_test.go:168: expected gazelle exit code: 0
got: 1
python_test.go:178: expected gazelle stderr: got: gazelle: ERROR: failed to validate dependencies for target "//bar:py_default_library":
"bar/bar.py", line 1: multiple targets (//foo:script, //foo:py_default_library) may be imported with "foo.script": possible solutions:
1. Disambiguate the above multiple targets by removing duplicate srcs entries.
2. Use the '# gazelle:resolve py foo.script TARGET_LABEL' BUILD file directive to resolve to one of the above targets.
"bar/bar.py", line 1: "foo" is an invalid dependency: possible solutions:
1. Add it as a dependency in the requirements.txt file.
2. Use the '# gazelle:resolve py foo TARGET_LABEL' BUILD file directive to resolve to a known dependency.
3. Ignore it with a comment '# gazelle:ignore foo' in the Python file.
FAIL
================================================================================

linzhp · 2025-07-14T15:25:24Z

PTAL

linzhp · 2025-06-15T04:29:16Z

This is yet another implementation of #2822. Based on this:

rules_python/gazelle/python/generate.go

Lines 248 to 252 in 94e08f7

// Remove the file from srcs if we're doing per-file library generation so
// that we don't also generate a py_library target for it.
ifcfg.PerFileGeneration() { 
srcs.Remove(name) 
 } 

The only case when a python file does not exist in py_library is in file mode. Stop indexing py_binary in other mode to avoid ambiguous dependency resolution.

linzhp · 2025-06-23T04:52:03Z

This PR is ready for review. The failing CI is an infra issue

linzhp · 2025-06-29T15:05:03Z

Added tests. Please take another look

linzhp · 2025-07-13T17:43:06Z

@aignas friendly ping...

dougthor42

I tested this on our big repo that uses file mode; looks good (eg gazelle didn't cause any unexpected changes).

Request: please update CHANGELOG.md with a fixed entry.

For my own future reference, this is what happens without the fix (eg reverting e9bd803):

=================== Test output for //python:python_test_import_with_same_srcs_library_and_binary:
--- FAIL: TestGazelleBinary (0.00s)
--- FAIL: TestGazelleBinary/import_with_same_srcs_library_and_binary (0.03s)
python_test.go:168: expected gazelle exit code: 0
got: 1
python_test.go:178: expected gazelle stderr: got: gazelle: ERROR: failed to validate dependencies for target "//bar:py_default_library":
"bar/bar.py", line 1: multiple targets (//foo:script, //foo:py_default_library) may be imported with "foo.script": possible solutions:
1. Disambiguate the above multiple targets by removing duplicate srcs entries.
2. Use the '# gazelle:resolve py foo.script TARGET_LABEL' BUILD file directive to resolve to one of the above targets.
"bar/bar.py", line 1: "foo" is an invalid dependency: possible solutions:
1. Add it as a dependency in the requirements.txt file.
2. Use the '# gazelle:resolve py foo TARGET_LABEL' BUILD file directive to resolve to a known dependency.
3. Ignore it with a comment '# gazelle:ignore foo' in the Python file.
FAIL
================================================================================

linzhp · 2025-07-14T15:25:24Z

PTAL

linzhp · 2025-06-15T04:29:16Z

This is yet another implementation of #2822. Based on this:

rules_python/gazelle/python/generate.go

Lines 248 to 252 in 94e08f7

// Remove the file from srcs if we're doing per-file library generation so
// that we don't also generate a py_library target for it.
ifcfg.PerFileGeneration() { 
srcs.Remove(name) 
 } 

The only case when a python file does not exist in py_library is in file mode. Stop indexing py_binary in other mode to avoid ambiguous dependency resolution.

linzhp · 2025-06-23T04:52:03Z

This PR is ready for review. The failing CI is an infra issue

linzhp · 2025-06-29T15:05:03Z

Added tests. Please take another look

linzhp · 2025-07-13T17:43:06Z

@aignas friendly ping...

dougthor42

I tested this on our big repo that uses file mode; looks good (eg gazelle didn't cause any unexpected changes).

Request: please update CHANGELOG.md with a fixed entry.

For my own future reference, this is what happens without the fix (eg reverting e9bd803):

=================== Test output for //python:python_test_import_with_same_srcs_library_and_binary:
--- FAIL: TestGazelleBinary (0.00s)
--- FAIL: TestGazelleBinary/import_with_same_srcs_library_and_binary (0.03s)
python_test.go:168: expected gazelle exit code: 0
got: 1
python_test.go:178: expected gazelle stderr: got: gazelle: ERROR: failed to validate dependencies for target "//bar:py_default_library":
"bar/bar.py", line 1: multiple targets (//foo:script, //foo:py_default_library) may be imported with "foo.script": possible solutions:
1. Disambiguate the above multiple targets by removing duplicate srcs entries.
2. Use the '# gazelle:resolve py foo.script TARGET_LABEL' BUILD file directive to resolve to one of the above targets.
"bar/bar.py", line 1: "foo" is an invalid dependency: possible solutions:
1. Add it as a dependency in the requirements.txt file.
2. Use the '# gazelle:resolve py foo TARGET_LABEL' BUILD file directive to resolve to a known dependency.
3. Ignore it with a comment '# gazelle:ignore foo' in the Python file.
FAIL
================================================================================

linzhp · 2025-07-14T15:25:24Z

PTAL

linzhp · 2025-06-15T04:29:16Z

This is yet another implementation of #2822. Based on this:

rules_python/gazelle/python/generate.go

Lines 248 to 252 in 94e08f7

// Remove the file from srcs if we're doing per-file library generation so
// that we don't also generate a py_library target for it.
ifcfg.PerFileGeneration() { 
srcs.Remove(name) 
 } 

The only case when a python file does not exist in py_library is in file mode. Stop indexing py_binary in other mode to avoid ambiguous dependency resolution.

linzhp · 2025-06-23T04:52:03Z

This PR is ready for review. The failing CI is an infra issue

linzhp · 2025-06-29T15:05:03Z

Added tests. Please take another look

linzhp · 2025-07-13T17:43:06Z

@aignas friendly ping...

dougthor42

I tested this on our big repo that uses file mode; looks good (eg gazelle didn't cause any unexpected changes).

Request: please update CHANGELOG.md with a fixed entry.

For my own future reference, this is what happens without the fix (eg reverting e9bd803):

=================== Test output for //python:python_test_import_with_same_srcs_library_and_binary:
--- FAIL: TestGazelleBinary (0.00s)
--- FAIL: TestGazelleBinary/import_with_same_srcs_library_and_binary (0.03s)
python_test.go:168: expected gazelle exit code: 0
got: 1
python_test.go:178: expected gazelle stderr: got: gazelle: ERROR: failed to validate dependencies for target "//bar:py_default_library":
"bar/bar.py", line 1: multiple targets (//foo:script, //foo:py_default_library) may be imported with "foo.script": possible solutions:
1. Disambiguate the above multiple targets by removing duplicate srcs entries.
2. Use the '# gazelle:resolve py foo.script TARGET_LABEL' BUILD file directive to resolve to one of the above targets.
"bar/bar.py", line 1: "foo" is an invalid dependency: possible solutions:
1. Add it as a dependency in the requirements.txt file.
2. Use the '# gazelle:resolve py foo TARGET_LABEL' BUILD file directive to resolve to a known dependency.
3. Ignore it with a comment '# gazelle:ignore foo' in the Python file.
FAIL
================================================================================

linzhp · 2025-07-14T15:25:24Z

PTAL

linzhp · 2025-06-15T04:29:16Z

This is yet another implementation of #2822. Based on this:

rules_python/gazelle/python/generate.go

Lines 248 to 252 in 94e08f7

// Remove the file from srcs if we're doing per-file library generation so
// that we don't also generate a py_library target for it.
ifcfg.PerFileGeneration() { 
srcs.Remove(name) 
 } 

The only case when a python file does not exist in py_library is in file mode. Stop indexing py_binary in other mode to avoid ambiguous dependency resolution.

linzhp · 2025-06-23T04:52:03Z

This PR is ready for review. The failing CI is an infra issue

linzhp · 2025-06-29T15:05:03Z

Added tests. Please take another look

linzhp · 2025-07-13T17:43:06Z

@aignas friendly ping...

dougthor42

I tested this on our big repo that uses file mode; looks good (eg gazelle didn't cause any unexpected changes).

Request: please update CHANGELOG.md with a fixed entry.

For my own future reference, this is what happens without the fix (eg reverting e9bd803):

=================== Test output for //python:python_test_import_with_same_srcs_library_and_binary:
--- FAIL: TestGazelleBinary (0.00s)
--- FAIL: TestGazelleBinary/import_with_same_srcs_library_and_binary (0.03s)
python_test.go:168: expected gazelle exit code: 0
got: 1
python_test.go:178: expected gazelle stderr: got: gazelle: ERROR: failed to validate dependencies for target "//bar:py_default_library":
"bar/bar.py", line 1: multiple targets (//foo:script, //foo:py_default_library) may be imported with "foo.script": possible solutions:
1. Disambiguate the above multiple targets by removing duplicate srcs entries.
2. Use the '# gazelle:resolve py foo.script TARGET_LABEL' BUILD file directive to resolve to one of the above targets.
"bar/bar.py", line 1: "foo" is an invalid dependency: possible solutions:
1. Add it as a dependency in the requirements.txt file.
2. Use the '# gazelle:resolve py foo TARGET_LABEL' BUILD file directive to resolve to a known dependency.
3. Ignore it with a comment '# gazelle:ignore foo' in the Python file.
FAIL
================================================================================

linzhp · 2025-07-14T15:25:24Z

PTAL

linzhp · 2025-06-15T04:29:16Z

This is yet another implementation of #2822. Based on this:

rules_python/gazelle/python/generate.go

Lines 248 to 252 in 94e08f7

// Remove the file from srcs if we're doing per-file library generation so
// that we don't also generate a py_library target for it.
ifcfg.PerFileGeneration() { 
srcs.Remove(name) 
 } 

The only case when a python file does not exist in py_library is in file mode. Stop indexing py_binary in other mode to avoid ambiguous dependency resolution.

linzhp · 2025-06-23T04:52:03Z

This PR is ready for review. The failing CI is an infra issue

linzhp · 2025-06-29T15:05:03Z

Added tests. Please take another look

linzhp · 2025-07-13T17:43:06Z

@aignas friendly ping...

dougthor42

I tested this on our big repo that uses file mode; looks good (eg gazelle didn't cause any unexpected changes).

Request: please update CHANGELOG.md with a fixed entry.

For my own future reference, this is what happens without the fix (eg reverting e9bd803):

=================== Test output for //python:python_test_import_with_same_srcs_library_and_binary:
--- FAIL: TestGazelleBinary (0.00s)
--- FAIL: TestGazelleBinary/import_with_same_srcs_library_and_binary (0.03s)
python_test.go:168: expected gazelle exit code: 0
got: 1
python_test.go:178: expected gazelle stderr: got: gazelle: ERROR: failed to validate dependencies for target "//bar:py_default_library":
"bar/bar.py", line 1: multiple targets (//foo:script, //foo:py_default_library) may be imported with "foo.script": possible solutions:
1. Disambiguate the above multiple targets by removing duplicate srcs entries.
2. Use the '# gazelle:resolve py foo.script TARGET_LABEL' BUILD file directive to resolve to one of the above targets.
"bar/bar.py", line 1: "foo" is an invalid dependency: possible solutions:
1. Add it as a dependency in the requirements.txt file.
2. Use the '# gazelle:resolve py foo TARGET_LABEL' BUILD file directive to resolve to a known dependency.
3. Ignore it with a comment '# gazelle:ignore foo' in the Python file.
FAIL
================================================================================

linzhp · 2025-07-14T15:25:24Z

PTAL

linzhp · 2025-06-15T04:29:16Z

This is yet another implementation of #2822. Based on this:

rules_python/gazelle/python/generate.go

Lines 248 to 252 in 94e08f7

// Remove the file from srcs if we're doing per-file library generation so
// that we don't also generate a py_library target for it.
ifcfg.PerFileGeneration() { 
srcs.Remove(name) 
 } 

The only case when a python file does not exist in py_library is in file mode. Stop indexing py_binary in other mode to avoid ambiguous dependency resolution.

linzhp · 2025-06-23T04:52:03Z

This PR is ready for review. The failing CI is an infra issue

linzhp · 2025-06-29T15:05:03Z

Added tests. Please take another look

linzhp · 2025-07-13T17:43:06Z

@aignas friendly ping...

dougthor42

I tested this on our big repo that uses file mode; looks good (eg gazelle didn't cause any unexpected changes).

Request: please update CHANGELOG.md with a fixed entry.

For my own future reference, this is what happens without the fix (eg reverting e9bd803):

=================== Test output for //python:python_test_import_with_same_srcs_library_and_binary:
--- FAIL: TestGazelleBinary (0.00s)
--- FAIL: TestGazelleBinary/import_with_same_srcs_library_and_binary (0.03s)
python_test.go:168: expected gazelle exit code: 0
got: 1
python_test.go:178: expected gazelle stderr: got: gazelle: ERROR: failed to validate dependencies for target "//bar:py_default_library":
"bar/bar.py", line 1: multiple targets (//foo:script, //foo:py_default_library) may be imported with "foo.script": possible solutions:
1. Disambiguate the above multiple targets by removing duplicate srcs entries.
2. Use the '# gazelle:resolve py foo.script TARGET_LABEL' BUILD file directive to resolve to one of the above targets.
"bar/bar.py", line 1: "foo" is an invalid dependency: possible solutions:
1. Add it as a dependency in the requirements.txt file.
2. Use the '# gazelle:resolve py foo TARGET_LABEL' BUILD file directive to resolve to a known dependency.
3. Ignore it with a comment '# gazelle:ignore foo' in the Python file.
FAIL
================================================================================

linzhp · 2025-07-14T15:25:24Z

PTAL

linzhp · 2025-06-15T04:29:16Z

This is yet another implementation of #2822. Based on this:

rules_python/gazelle/python/generate.go

Lines 248 to 252 in 94e08f7

// Remove the file from srcs if we're doing per-file library generation so
// that we don't also generate a py_library target for it.
ifcfg.PerFileGeneration() { 
srcs.Remove(name) 
 } 

The only case when a python file does not exist in py_library is in file mode. Stop indexing py_binary in other mode to avoid ambiguous dependency resolution.

linzhp · 2025-06-23T04:52:03Z

This PR is ready for review. The failing CI is an infra issue

linzhp · 2025-06-29T15:05:03Z

Added tests. Please take another look

linzhp · 2025-07-13T17:43:06Z

@aignas friendly ping...

dougthor42

I tested this on our big repo that uses file mode; looks good (eg gazelle didn't cause any unexpected changes).

Request: please update CHANGELOG.md with a fixed entry.

For my own future reference, this is what happens without the fix (eg reverting e9bd803):

=================== Test output for //python:python_test_import_with_same_srcs_library_and_binary:
--- FAIL: TestGazelleBinary (0.00s)
--- FAIL: TestGazelleBinary/import_with_same_srcs_library_and_binary (0.03s)
python_test.go:168: expected gazelle exit code: 0
got: 1
python_test.go:178: expected gazelle stderr: got: gazelle: ERROR: failed to validate dependencies for target "//bar:py_default_library":
"bar/bar.py", line 1: multiple targets (//foo:script, //foo:py_default_library) may be imported with "foo.script": possible solutions:
1. Disambiguate the above multiple targets by removing duplicate srcs entries.
2. Use the '# gazelle:resolve py foo.script TARGET_LABEL' BUILD file directive to resolve to one of the above targets.
"bar/bar.py", line 1: "foo" is an invalid dependency: possible solutions:
1. Add it as a dependency in the requirements.txt file.
2. Use the '# gazelle:resolve py foo TARGET_LABEL' BUILD file directive to resolve to a known dependency.
3. Ignore it with a comment '# gazelle:ignore foo' in the Python file.
FAIL
================================================================================

linzhp · 2025-07-14T15:25:24Z

PTAL

do not index py_binary unless in file mode
e9bd803

linzhp requested review from aignas and dougthor42 as code owners June 15, 2025 04:29

linzhp mentioned this pull request Jun 15, 2025
feat!: always generating py_library #2982
Draft

aignas reviewed Jun 24, 2025
View reviewed changes

Comment threadgazelle/python/resolve.go

Adding tests
a070e03

dougthor42 reviewed Jul 14, 2025
View reviewed changes

linzhp added 2 commits July 14, 2025 08:14

addressing comments
a3ee5c1

Merge remote-tracking branch 'upstream/main' into py_binary2
b964e25

do not index py_binary unless in file mode
e9bd803

linzhp requested review from aignas and dougthor42 as code owners June 15, 2025 04:29

linzhp mentioned this pull request Jun 15, 2025
feat!: always generating py_library #2982
Draft

aignas reviewed Jun 24, 2025
View reviewed changes

Comment threadgazelle/python/resolve.go

Adding tests
a070e03

dougthor42 reviewed Jul 14, 2025
View reviewed changes

linzhp added 2 commits July 14, 2025 08:14

addressing comments
a3ee5c1

Merge remote-tracking branch 'upstream/main' into py_binary2
b964e25

do not index py_binary unless in file mode
e9bd803

linzhp requested review from aignas and dougthor42 as code owners June 15, 2025 04:29

linzhp mentioned this pull request Jun 15, 2025
feat!: always generating py_library #2982
Draft

aignas reviewed Jun 24, 2025
View reviewed changes

Comment threadgazelle/python/resolve.go

Adding tests
a070e03

dougthor42 reviewed Jul 14, 2025
View reviewed changes

linzhp added 2 commits July 14, 2025 08:14

addressing comments
a3ee5c1

Merge remote-tracking branch 'upstream/main' into py_binary2
b964e25

do not index py_binary unless in file mode
e9bd803

linzhp requested review from aignas and dougthor42 as code owners June 15, 2025 04:29

linzhp mentioned this pull request Jun 15, 2025
feat!: always generating py_library #2982
Draft

aignas reviewed Jun 24, 2025
View reviewed changes

Comment threadgazelle/python/resolve.go

Adding tests
a070e03

dougthor42 reviewed Jul 14, 2025
View reviewed changes

linzhp added 2 commits July 14, 2025 08:14

addressing comments
a3ee5c1

Merge remote-tracking branch 'upstream/main' into py_binary2
b964e25

do not index py_binary unless in file mode
e9bd803

linzhp requested review from aignas and dougthor42 as code owners June 15, 2025 04:29

linzhp mentioned this pull request Jun 15, 2025
feat!: always generating py_library #2982
Draft

aignas reviewed Jun 24, 2025
View reviewed changes

Comment threadgazelle/python/resolve.go

Adding tests
a070e03

dougthor42 reviewed Jul 14, 2025
View reviewed changes

linzhp added 2 commits July 14, 2025 08:14

addressing comments
a3ee5c1

Merge remote-tracking branch 'upstream/main' into py_binary2
b964e25

do not index py_binary unless in file mode
e9bd803

linzhp requested review from aignas and dougthor42 as code owners June 15, 2025 04:29

linzhp mentioned this pull request Jun 15, 2025
feat!: always generating py_library #2982
Draft

aignas reviewed Jun 24, 2025
View reviewed changes

Comment threadgazelle/python/resolve.go

Adding tests
a070e03

dougthor42 reviewed Jul 14, 2025
View reviewed changes

linzhp added 2 commits July 14, 2025 08:14

addressing comments
a3ee5c1

Merge remote-tracking branch 'upstream/main' into py_binary2
b964e25

do not index py_binary unless in file mode
e9bd803

linzhp requested review from aignas and dougthor42 as code owners June 15, 2025 04:29

linzhp mentioned this pull request Jun 15, 2025
feat!: always generating py_library #2982
Draft

aignas reviewed Jun 24, 2025
View reviewed changes

Comment threadgazelle/python/resolve.go

Adding tests
a070e03

dougthor42 reviewed Jul 14, 2025
View reviewed changes

linzhp added 2 commits July 14, 2025 08:14

addressing comments
a3ee5c1

Merge remote-tracking branch 'upstream/main' into py_binary2
b964e25

do not index py_binary unless in file mode
e9bd803

linzhp requested review from aignas and dougthor42 as code owners June 15, 2025 04:29

linzhp mentioned this pull request Jun 15, 2025
feat!: always generating py_library #2982
Draft

aignas reviewed Jun 24, 2025
View reviewed changes

Comment threadgazelle/python/resolve.go

Adding tests
a070e03

dougthor42 reviewed Jul 14, 2025
View reviewed changes

linzhp added 2 commits July 14, 2025 08:14

addressing comments
a3ee5c1

Merge remote-tracking branch 'upstream/main' into py_binary2
b964e25

	// Remove the file from srcs if we're doing per-file library generation so
	// that we don't also generate a py_library target for it.
	ifcfg.PerFileGeneration() {
	srcs.Remove(name)
	}

Uh oh!

Uh oh!

Conversation

linzhp commented Jun 15, 2025

Uh oh!

linzhp commented Jun 23, 2025

Uh oh!

Uh oh!

linzhp commented Jun 29, 2025

Uh oh!

linzhp commented Jul 13, 2025

Uh oh!

dougthor42 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

linzhp commented Jul 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

linzhp commented Jun 15, 2025

Uh oh!

linzhp commented Jun 23, 2025

Uh oh!

Uh oh!

linzhp commented Jun 29, 2025

Uh oh!

linzhp commented Jul 13, 2025

Uh oh!

dougthor42 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

linzhp commented Jul 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

linzhp commented Jun 15, 2025

Uh oh!

linzhp commented Jun 23, 2025

Uh oh!

Uh oh!

linzhp commented Jun 29, 2025

Uh oh!

linzhp commented Jul 13, 2025

Uh oh!

dougthor42 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

linzhp commented Jul 14, 2025

Uh oh!

Reviewers