Skip to content

fix(gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcs - #2822

Closed
yushan26 wants to merge 5 commits into
bazel-contrib:mainfrom
yushan26:index-py-library
Closed

fix(gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcs#2822
yushan26 wants to merge 5 commits into
bazel-contrib:mainfrom
yushan26:index-py-library

Conversation

@yushan26

@yushan26yushan26 commented Apr 24, 2025

Copy link
Copy Markdown
Contributor

This PR ensures that py_binary rules are not indexed into Gazelle's IndexMap when there is a corresponding py_library rule with the same srcs. When both py_library and py_binary targets share the same file in their srcs, Gazelle previously indexed both under the same import path. This led to ambiguity and resolution errors, as Gazelle found multiple rules for the same language (py).

To resolve this, the PR updates Gazelle to skip indexing py_binary rules, allowing py_library to be the only import being indexed.

Testing
An integration test was added where both a py_library and a py_binary rule include the same script.py in their srcs. The test verifies that Gazelle correctly resolves the import to the py_library target.

@arrdem

arrdem commented May 7, 2025

Copy link
Copy Markdown
Contributor

I don't think that just not indexing py_binary sources is a workable solution. A pattern we've seen before is that users will want to consider sources part of a py_binary and explicitly not want to manage what's perceived to be a duplicate py_library target, instead expecting other use sites to depend on that fileset via the py_binary target.

This has obvious downsides in that the entrypoint script gets materialized among other things, but it's an existing and working use-case.

I think the behavior in this case should simply be to prefer the library target in case both are an option, not ignoring binaries entirely.

@linzhp

Copy link
Copy Markdown
Contributor

A pattern we've seen before is that users will want to consider sources part of a py_binary and explicitly not want to manage what's perceived to be a duplicate

Are you talking about the scenarios when py_binary is generated but not py_library?

@yushan26yushan26 changed the title Skip indexing py_binary rulesSkip indexing py_binary rules if a corresponding py_library rule contains the same srcsMay 8, 2025
@yushan26

Copy link
Copy Markdown
ContributorAuthor

Gotcha, I updated the logic to skip indexing the py_binary rule only if there is a corresponding py_library rule that also has the samesrcs populated. Let me know if this makes sense.

@yushan26yushan26 changed the title Skip indexing py_binary rules if a corresponding py_library rule contains the same srcsfix(gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcsMay 27, 2025
@yushan26
yushan26 requested a review from rickeylev as a code ownerMay 27, 2025 20:39
@linzhp

Copy link
Copy Markdown
Contributor

@dougthor42 can you also take a look at this PR?

One thing I am not sure is whether Gazelle can generate py_binary without generating py_library like @arrdem said.

@dougthor42dougthor42 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I am not sure is whether Gazelle can generate py_binary without generating py_library.

In general, yes this is possible. Given

# foo.pyif__name__=="__main__":
print("hey")

Then, assuming gazelle:python_generation_mode file, Gazelle will generate:

py_binary(
name="foo",
srcs= ["foo.py"],
)

However, package and project generation modes are different and TBH I'm less familiar with them.

Comment threadCHANGELOG.md Outdated
multiple times.
* (tools/wheelmaker.py) Extras are now preserved in Requires-Dist metadata when using requires_file
to specify the requirements.
* (gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcs: https://github.com/bazel-contrib/rules_python/pull/2822

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrap at ~80chars please

Comment threadgazelle/python/resolve.go Outdated
}
pyLibrarySrcs := otherRule.AttrStrings("srcs")
for _, src := range pyLibrarySrcs {
if src == pyBinarySrcs[0] {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A py_binary can have multiple srcs so you don't want to check against only the 1st value.

What are the requirements for not indexing the binary?

  • Any binary source file is found in any py_library srcs?
  • All binary srcs are found in a single py_library? Eg binary.srcs is a subset of library.srcs.
  • All binary srcs are found in multiple py_library targets?

@linzhp

Copy link
Copy Markdown
Contributor

Should we make Gazelle always generate py_library and stop indexing any py_binary? This is more consistent with other languages.

@yushan26

Copy link
Copy Markdown
ContributorAuthor

Should we make Gazelle always generate py_library and stop indexing any py_binary? This is more consistent with other languages.

Yea that's probably more consistent, although this may be unexpected behavior to the existing gazelle extension.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@yushan26@arrdem@linzhp@dougthor42@yushan8
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
fix(gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcs by yushan26 · Pull Request #2822 · bazel-contrib/rules_python · GitHub
Skip to content

fix(gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcs - #2822

Closed
yushan26 wants to merge 5 commits into
bazel-contrib:mainfrom
yushan26:index-py-library
Closed

fix(gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcs#2822
yushan26 wants to merge 5 commits into
bazel-contrib:mainfrom
yushan26:index-py-library

Conversation

@yushan26

@yushan26yushan26 commented Apr 24, 2025

Copy link
Copy Markdown
Contributor

This PR ensures that py_binary rules are not indexed into Gazelle's IndexMap when there is a corresponding py_library rule with the same srcs. When both py_library and py_binary targets share the same file in their srcs, Gazelle previously indexed both under the same import path. This led to ambiguity and resolution errors, as Gazelle found multiple rules for the same language (py).

To resolve this, the PR updates Gazelle to skip indexing py_binary rules, allowing py_library to be the only import being indexed.

Testing
An integration test was added where both a py_library and a py_binary rule include the same script.py in their srcs. The test verifies that Gazelle correctly resolves the import to the py_library target.

@arrdem

arrdem commented May 7, 2025

Copy link
Copy Markdown
Contributor

I don't think that just not indexing py_binary sources is a workable solution. A pattern we've seen before is that users will want to consider sources part of a py_binary and explicitly not want to manage what's perceived to be a duplicate py_library target, instead expecting other use sites to depend on that fileset via the py_binary target.

This has obvious downsides in that the entrypoint script gets materialized among other things, but it's an existing and working use-case.

I think the behavior in this case should simply be to prefer the library target in case both are an option, not ignoring binaries entirely.

@linzhp

Copy link
Copy Markdown
Contributor

A pattern we've seen before is that users will want to consider sources part of a py_binary and explicitly not want to manage what's perceived to be a duplicate

Are you talking about the scenarios when py_binary is generated but not py_library?

@yushan26yushan26 changed the title Skip indexing py_binary rulesSkip indexing py_binary rules if a corresponding py_library rule contains the same srcsMay 8, 2025
@yushan26

Copy link
Copy Markdown
ContributorAuthor

Gotcha, I updated the logic to skip indexing the py_binary rule only if there is a corresponding py_library rule that also has the samesrcs populated. Let me know if this makes sense.

@yushan26yushan26 changed the title Skip indexing py_binary rules if a corresponding py_library rule contains the same srcsfix(gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcsMay 27, 2025
@yushan26
yushan26 requested a review from rickeylev as a code ownerMay 27, 2025 20:39
@linzhp

Copy link
Copy Markdown
Contributor

@dougthor42 can you also take a look at this PR?

One thing I am not sure is whether Gazelle can generate py_binary without generating py_library like @arrdem said.

@dougthor42dougthor42 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I am not sure is whether Gazelle can generate py_binary without generating py_library.

In general, yes this is possible. Given

# foo.pyif__name__=="__main__":
print("hey")

Then, assuming gazelle:python_generation_mode file, Gazelle will generate:

py_binary(
name="foo",
srcs= ["foo.py"],
)

However, package and project generation modes are different and TBH I'm less familiar with them.

Comment threadCHANGELOG.md Outdated
multiple times.
* (tools/wheelmaker.py) Extras are now preserved in Requires-Dist metadata when using requires_file
to specify the requirements.
* (gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcs: https://github.com/bazel-contrib/rules_python/pull/2822

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrap at ~80chars please

Comment threadgazelle/python/resolve.go Outdated
}
pyLibrarySrcs := otherRule.AttrStrings("srcs")
for _, src := range pyLibrarySrcs {
if src == pyBinarySrcs[0] {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A py_binary can have multiple srcs so you don't want to check against only the 1st value.

What are the requirements for not indexing the binary?

  • Any binary source file is found in any py_library srcs?
  • All binary srcs are found in a single py_library? Eg binary.srcs is a subset of library.srcs.
  • All binary srcs are found in multiple py_library targets?

@linzhp

Copy link
Copy Markdown
Contributor

Should we make Gazelle always generate py_library and stop indexing any py_binary? This is more consistent with other languages.

@yushan26

Copy link
Copy Markdown
ContributorAuthor

Should we make Gazelle always generate py_library and stop indexing any py_binary? This is more consistent with other languages.

Yea that's probably more consistent, although this may be unexpected behavior to the existing gazelle extension.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@yushan26@arrdem@linzhp@dougthor42@yushan8
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcs by yushan26 · Pull Request #2822 · bazel-contrib/rules_python · GitHub
Skip to content

fix(gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcs - #2822

Closed
yushan26 wants to merge 5 commits into
bazel-contrib:mainfrom
yushan26:index-py-library
Closed

fix(gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcs#2822
yushan26 wants to merge 5 commits into
bazel-contrib:mainfrom
yushan26:index-py-library

Conversation

@yushan26

@yushan26yushan26 commented Apr 24, 2025

Copy link
Copy Markdown
Contributor

This PR ensures that py_binary rules are not indexed into Gazelle's IndexMap when there is a corresponding py_library rule with the same srcs. When both py_library and py_binary targets share the same file in their srcs, Gazelle previously indexed both under the same import path. This led to ambiguity and resolution errors, as Gazelle found multiple rules for the same language (py).

To resolve this, the PR updates Gazelle to skip indexing py_binary rules, allowing py_library to be the only import being indexed.

Testing
An integration test was added where both a py_library and a py_binary rule include the same script.py in their srcs. The test verifies that Gazelle correctly resolves the import to the py_library target.

@arrdem

arrdem commented May 7, 2025

Copy link
Copy Markdown
Contributor

I don't think that just not indexing py_binary sources is a workable solution. A pattern we've seen before is that users will want to consider sources part of a py_binary and explicitly not want to manage what's perceived to be a duplicate py_library target, instead expecting other use sites to depend on that fileset via the py_binary target.

This has obvious downsides in that the entrypoint script gets materialized among other things, but it's an existing and working use-case.

I think the behavior in this case should simply be to prefer the library target in case both are an option, not ignoring binaries entirely.

@linzhp

Copy link
Copy Markdown
Contributor

A pattern we've seen before is that users will want to consider sources part of a py_binary and explicitly not want to manage what's perceived to be a duplicate

Are you talking about the scenarios when py_binary is generated but not py_library?

@yushan26yushan26 changed the title Skip indexing py_binary rulesSkip indexing py_binary rules if a corresponding py_library rule contains the same srcsMay 8, 2025
@yushan26

Copy link
Copy Markdown
ContributorAuthor

Gotcha, I updated the logic to skip indexing the py_binary rule only if there is a corresponding py_library rule that also has the samesrcs populated. Let me know if this makes sense.

@yushan26yushan26 changed the title Skip indexing py_binary rules if a corresponding py_library rule contains the same srcsfix(gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcsMay 27, 2025
@yushan26
yushan26 requested a review from rickeylev as a code ownerMay 27, 2025 20:39
@linzhp

Copy link
Copy Markdown
Contributor

@dougthor42 can you also take a look at this PR?

One thing I am not sure is whether Gazelle can generate py_binary without generating py_library like @arrdem said.

@dougthor42dougthor42 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I am not sure is whether Gazelle can generate py_binary without generating py_library.

In general, yes this is possible. Given

# foo.pyif__name__=="__main__":
print("hey")

Then, assuming gazelle:python_generation_mode file, Gazelle will generate:

py_binary(
name="foo",
srcs= ["foo.py"],
)

However, package and project generation modes are different and TBH I'm less familiar with them.

Comment threadCHANGELOG.md Outdated
multiple times.
* (tools/wheelmaker.py) Extras are now preserved in Requires-Dist metadata when using requires_file
to specify the requirements.
* (gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcs: https://github.com/bazel-contrib/rules_python/pull/2822

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrap at ~80chars please

Comment threadgazelle/python/resolve.go Outdated
}
pyLibrarySrcs := otherRule.AttrStrings("srcs")
for _, src := range pyLibrarySrcs {
if src == pyBinarySrcs[0] {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A py_binary can have multiple srcs so you don't want to check against only the 1st value.

What are the requirements for not indexing the binary?

  • Any binary source file is found in any py_library srcs?
  • All binary srcs are found in a single py_library? Eg binary.srcs is a subset of library.srcs.
  • All binary srcs are found in multiple py_library targets?

@linzhp

Copy link
Copy Markdown
Contributor

Should we make Gazelle always generate py_library and stop indexing any py_binary? This is more consistent with other languages.

@yushan26

Copy link
Copy Markdown
ContributorAuthor

Should we make Gazelle always generate py_library and stop indexing any py_binary? This is more consistent with other languages.

Yea that's probably more consistent, although this may be unexpected behavior to the existing gazelle extension.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@yushan26@arrdem@linzhp@dougthor42@yushan8
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcs by yushan26 · Pull Request #2822 · bazel-contrib/rules_python · GitHub
Skip to content

fix(gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcs - #2822

Closed
yushan26 wants to merge 5 commits into
bazel-contrib:mainfrom
yushan26:index-py-library
Closed

fix(gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcs#2822
yushan26 wants to merge 5 commits into
bazel-contrib:mainfrom
yushan26:index-py-library

Conversation

@yushan26

@yushan26yushan26 commented Apr 24, 2025

Copy link
Copy Markdown
Contributor

This PR ensures that py_binary rules are not indexed into Gazelle's IndexMap when there is a corresponding py_library rule with the same srcs. When both py_library and py_binary targets share the same file in their srcs, Gazelle previously indexed both under the same import path. This led to ambiguity and resolution errors, as Gazelle found multiple rules for the same language (py).

To resolve this, the PR updates Gazelle to skip indexing py_binary rules, allowing py_library to be the only import being indexed.

Testing
An integration test was added where both a py_library and a py_binary rule include the same script.py in their srcs. The test verifies that Gazelle correctly resolves the import to the py_library target.

@arrdem

arrdem commented May 7, 2025

Copy link
Copy Markdown
Contributor

I don't think that just not indexing py_binary sources is a workable solution. A pattern we've seen before is that users will want to consider sources part of a py_binary and explicitly not want to manage what's perceived to be a duplicate py_library target, instead expecting other use sites to depend on that fileset via the py_binary target.

This has obvious downsides in that the entrypoint script gets materialized among other things, but it's an existing and working use-case.

I think the behavior in this case should simply be to prefer the library target in case both are an option, not ignoring binaries entirely.

@linzhp

Copy link
Copy Markdown
Contributor

A pattern we've seen before is that users will want to consider sources part of a py_binary and explicitly not want to manage what's perceived to be a duplicate

Are you talking about the scenarios when py_binary is generated but not py_library?

@yushan26yushan26 changed the title Skip indexing py_binary rulesSkip indexing py_binary rules if a corresponding py_library rule contains the same srcsMay 8, 2025
@yushan26

Copy link
Copy Markdown
ContributorAuthor

Gotcha, I updated the logic to skip indexing the py_binary rule only if there is a corresponding py_library rule that also has the samesrcs populated. Let me know if this makes sense.

@yushan26yushan26 changed the title Skip indexing py_binary rules if a corresponding py_library rule contains the same srcsfix(gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcsMay 27, 2025
@yushan26
yushan26 requested a review from rickeylev as a code ownerMay 27, 2025 20:39
@linzhp

Copy link
Copy Markdown
Contributor

@dougthor42 can you also take a look at this PR?

One thing I am not sure is whether Gazelle can generate py_binary without generating py_library like @arrdem said.

@dougthor42dougthor42 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I am not sure is whether Gazelle can generate py_binary without generating py_library.

In general, yes this is possible. Given

# foo.pyif__name__=="__main__":
print("hey")

Then, assuming gazelle:python_generation_mode file, Gazelle will generate:

py_binary(
name="foo",
srcs= ["foo.py"],
)

However, package and project generation modes are different and TBH I'm less familiar with them.

Comment threadCHANGELOG.md Outdated
multiple times.
* (tools/wheelmaker.py) Extras are now preserved in Requires-Dist metadata when using requires_file
to specify the requirements.
* (gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcs: https://github.com/bazel-contrib/rules_python/pull/2822

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrap at ~80chars please

Comment threadgazelle/python/resolve.go Outdated
}
pyLibrarySrcs := otherRule.AttrStrings("srcs")
for _, src := range pyLibrarySrcs {
if src == pyBinarySrcs[0] {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A py_binary can have multiple srcs so you don't want to check against only the 1st value.

What are the requirements for not indexing the binary?

  • Any binary source file is found in any py_library srcs?
  • All binary srcs are found in a single py_library? Eg binary.srcs is a subset of library.srcs.
  • All binary srcs are found in multiple py_library targets?

@linzhp

Copy link
Copy Markdown
Contributor

Should we make Gazelle always generate py_library and stop indexing any py_binary? This is more consistent with other languages.

@yushan26

Copy link
Copy Markdown
ContributorAuthor

Should we make Gazelle always generate py_library and stop indexing any py_binary? This is more consistent with other languages.

Yea that's probably more consistent, although this may be unexpected behavior to the existing gazelle extension.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@yushan26@arrdem@linzhp@dougthor42@yushan8
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' fix(gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcs by yushan26 · Pull Request #2822 · bazel-contrib/rules_python · GitHub
Skip to content

fix(gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcs - #2822

Closed
yushan26 wants to merge 5 commits into
bazel-contrib:mainfrom
yushan26:index-py-library
Closed

fix(gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcs#2822
yushan26 wants to merge 5 commits into
bazel-contrib:mainfrom
yushan26:index-py-library

Conversation

@yushan26

@yushan26yushan26 commented Apr 24, 2025

Copy link
Copy Markdown
Contributor

This PR ensures that py_binary rules are not indexed into Gazelle's IndexMap when there is a corresponding py_library rule with the same srcs. When both py_library and py_binary targets share the same file in their srcs, Gazelle previously indexed both under the same import path. This led to ambiguity and resolution errors, as Gazelle found multiple rules for the same language (py).

To resolve this, the PR updates Gazelle to skip indexing py_binary rules, allowing py_library to be the only import being indexed.

Testing
An integration test was added where both a py_library and a py_binary rule include the same script.py in their srcs. The test verifies that Gazelle correctly resolves the import to the py_library target.

@arrdem

arrdem commented May 7, 2025

Copy link
Copy Markdown
Contributor

I don't think that just not indexing py_binary sources is a workable solution. A pattern we've seen before is that users will want to consider sources part of a py_binary and explicitly not want to manage what's perceived to be a duplicate py_library target, instead expecting other use sites to depend on that fileset via the py_binary target.

This has obvious downsides in that the entrypoint script gets materialized among other things, but it's an existing and working use-case.

I think the behavior in this case should simply be to prefer the library target in case both are an option, not ignoring binaries entirely.

@linzhp

Copy link
Copy Markdown
Contributor

A pattern we've seen before is that users will want to consider sources part of a py_binary and explicitly not want to manage what's perceived to be a duplicate

Are you talking about the scenarios when py_binary is generated but not py_library?

@yushan26yushan26 changed the title Skip indexing py_binary rulesSkip indexing py_binary rules if a corresponding py_library rule contains the same srcsMay 8, 2025
@yushan26

Copy link
Copy Markdown
ContributorAuthor

Gotcha, I updated the logic to skip indexing the py_binary rule only if there is a corresponding py_library rule that also has the samesrcs populated. Let me know if this makes sense.

@yushan26yushan26 changed the title Skip indexing py_binary rules if a corresponding py_library rule contains the same srcsfix(gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcsMay 27, 2025
@yushan26
yushan26 requested a review from rickeylev as a code ownerMay 27, 2025 20:39
@linzhp

Copy link
Copy Markdown
Contributor

@dougthor42 can you also take a look at this PR?

One thing I am not sure is whether Gazelle can generate py_binary without generating py_library like @arrdem said.

@dougthor42dougthor42 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I am not sure is whether Gazelle can generate py_binary without generating py_library.

In general, yes this is possible. Given

# foo.pyif__name__=="__main__":
print("hey")

Then, assuming gazelle:python_generation_mode file, Gazelle will generate:

py_binary(
name="foo",
srcs= ["foo.py"],
)

However, package and project generation modes are different and TBH I'm less familiar with them.

Comment threadCHANGELOG.md Outdated
multiple times.
* (tools/wheelmaker.py) Extras are now preserved in Requires-Dist metadata when using requires_file
to specify the requirements.
* (gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcs: https://github.com/bazel-contrib/rules_python/pull/2822

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrap at ~80chars please

Comment threadgazelle/python/resolve.go Outdated
}
pyLibrarySrcs := otherRule.AttrStrings("srcs")
for _, src := range pyLibrarySrcs {
if src == pyBinarySrcs[0] {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A py_binary can have multiple srcs so you don't want to check against only the 1st value.

What are the requirements for not indexing the binary?

  • Any binary source file is found in any py_library srcs?
  • All binary srcs are found in a single py_library? Eg binary.srcs is a subset of library.srcs.
  • All binary srcs are found in multiple py_library targets?

@linzhp

Copy link
Copy Markdown
Contributor

Should we make Gazelle always generate py_library and stop indexing any py_binary? This is more consistent with other languages.

@yushan26

Copy link
Copy Markdown
ContributorAuthor

Should we make Gazelle always generate py_library and stop indexing any py_binary? This is more consistent with other languages.

Yea that's probably more consistent, although this may be unexpected behavior to the existing gazelle extension.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@yushan26@arrdem@linzhp@dougthor42@yushan8
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcs by yushan26 · Pull Request #2822 · bazel-contrib/rules_python · GitHub
Skip to content

fix(gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcs - #2822

Closed
yushan26 wants to merge 5 commits into
bazel-contrib:mainfrom
yushan26:index-py-library
Closed

fix(gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcs#2822
yushan26 wants to merge 5 commits into
bazel-contrib:mainfrom
yushan26:index-py-library

Conversation

@yushan26

@yushan26yushan26 commented Apr 24, 2025

Copy link
Copy Markdown
Contributor

This PR ensures that py_binary rules are not indexed into Gazelle's IndexMap when there is a corresponding py_library rule with the same srcs. When both py_library and py_binary targets share the same file in their srcs, Gazelle previously indexed both under the same import path. This led to ambiguity and resolution errors, as Gazelle found multiple rules for the same language (py).

To resolve this, the PR updates Gazelle to skip indexing py_binary rules, allowing py_library to be the only import being indexed.

Testing
An integration test was added where both a py_library and a py_binary rule include the same script.py in their srcs. The test verifies that Gazelle correctly resolves the import to the py_library target.

@arrdem

arrdem commented May 7, 2025

Copy link
Copy Markdown
Contributor

I don't think that just not indexing py_binary sources is a workable solution. A pattern we've seen before is that users will want to consider sources part of a py_binary and explicitly not want to manage what's perceived to be a duplicate py_library target, instead expecting other use sites to depend on that fileset via the py_binary target.

This has obvious downsides in that the entrypoint script gets materialized among other things, but it's an existing and working use-case.

I think the behavior in this case should simply be to prefer the library target in case both are an option, not ignoring binaries entirely.

@linzhp

Copy link
Copy Markdown
Contributor

A pattern we've seen before is that users will want to consider sources part of a py_binary and explicitly not want to manage what's perceived to be a duplicate

Are you talking about the scenarios when py_binary is generated but not py_library?

@yushan26yushan26 changed the title Skip indexing py_binary rulesSkip indexing py_binary rules if a corresponding py_library rule contains the same srcsMay 8, 2025
@yushan26

Copy link
Copy Markdown
ContributorAuthor

Gotcha, I updated the logic to skip indexing the py_binary rule only if there is a corresponding py_library rule that also has the samesrcs populated. Let me know if this makes sense.

@yushan26yushan26 changed the title Skip indexing py_binary rules if a corresponding py_library rule contains the same srcsfix(gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcsMay 27, 2025
@yushan26
yushan26 requested a review from rickeylev as a code ownerMay 27, 2025 20:39
@linzhp

Copy link
Copy Markdown
Contributor

@dougthor42 can you also take a look at this PR?

One thing I am not sure is whether Gazelle can generate py_binary without generating py_library like @arrdem said.

@dougthor42dougthor42 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I am not sure is whether Gazelle can generate py_binary without generating py_library.

In general, yes this is possible. Given

# foo.pyif__name__=="__main__":
print("hey")

Then, assuming gazelle:python_generation_mode file, Gazelle will generate:

py_binary(
name="foo",
srcs= ["foo.py"],
)

However, package and project generation modes are different and TBH I'm less familiar with them.

Comment threadCHANGELOG.md Outdated
multiple times.
* (tools/wheelmaker.py) Extras are now preserved in Requires-Dist metadata when using requires_file
to specify the requirements.
* (gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcs: https://github.com/bazel-contrib/rules_python/pull/2822

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrap at ~80chars please

Comment threadgazelle/python/resolve.go Outdated
}
pyLibrarySrcs := otherRule.AttrStrings("srcs")
for _, src := range pyLibrarySrcs {
if src == pyBinarySrcs[0] {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A py_binary can have multiple srcs so you don't want to check against only the 1st value.

What are the requirements for not indexing the binary?

  • Any binary source file is found in any py_library srcs?
  • All binary srcs are found in a single py_library? Eg binary.srcs is a subset of library.srcs.
  • All binary srcs are found in multiple py_library targets?

@linzhp

Copy link
Copy Markdown
Contributor

Should we make Gazelle always generate py_library and stop indexing any py_binary? This is more consistent with other languages.

@yushan26

Copy link
Copy Markdown
ContributorAuthor

Should we make Gazelle always generate py_library and stop indexing any py_binary? This is more consistent with other languages.

Yea that's probably more consistent, although this may be unexpected behavior to the existing gazelle extension.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@yushan26@arrdem@linzhp@dougthor42@yushan8
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcs by yushan26 · Pull Request #2822 · bazel-contrib/rules_python · GitHub
Skip to content

fix(gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcs - #2822

Closed
yushan26 wants to merge 5 commits into
bazel-contrib:mainfrom
yushan26:index-py-library
Closed

fix(gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcs#2822
yushan26 wants to merge 5 commits into
bazel-contrib:mainfrom
yushan26:index-py-library

Conversation

@yushan26

@yushan26yushan26 commented Apr 24, 2025

Copy link
Copy Markdown
Contributor

This PR ensures that py_binary rules are not indexed into Gazelle's IndexMap when there is a corresponding py_library rule with the same srcs. When both py_library and py_binary targets share the same file in their srcs, Gazelle previously indexed both under the same import path. This led to ambiguity and resolution errors, as Gazelle found multiple rules for the same language (py).

To resolve this, the PR updates Gazelle to skip indexing py_binary rules, allowing py_library to be the only import being indexed.

Testing
An integration test was added where both a py_library and a py_binary rule include the same script.py in their srcs. The test verifies that Gazelle correctly resolves the import to the py_library target.

@arrdem

arrdem commented May 7, 2025

Copy link
Copy Markdown
Contributor

I don't think that just not indexing py_binary sources is a workable solution. A pattern we've seen before is that users will want to consider sources part of a py_binary and explicitly not want to manage what's perceived to be a duplicate py_library target, instead expecting other use sites to depend on that fileset via the py_binary target.

This has obvious downsides in that the entrypoint script gets materialized among other things, but it's an existing and working use-case.

I think the behavior in this case should simply be to prefer the library target in case both are an option, not ignoring binaries entirely.

@linzhp

Copy link
Copy Markdown
Contributor

A pattern we've seen before is that users will want to consider sources part of a py_binary and explicitly not want to manage what's perceived to be a duplicate

Are you talking about the scenarios when py_binary is generated but not py_library?

@yushan26yushan26 changed the title Skip indexing py_binary rulesSkip indexing py_binary rules if a corresponding py_library rule contains the same srcsMay 8, 2025
@yushan26

Copy link
Copy Markdown
ContributorAuthor

Gotcha, I updated the logic to skip indexing the py_binary rule only if there is a corresponding py_library rule that also has the samesrcs populated. Let me know if this makes sense.

@yushan26yushan26 changed the title Skip indexing py_binary rules if a corresponding py_library rule contains the same srcsfix(gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcsMay 27, 2025
@yushan26
yushan26 requested a review from rickeylev as a code ownerMay 27, 2025 20:39
@linzhp

Copy link
Copy Markdown
Contributor

@dougthor42 can you also take a look at this PR?

One thing I am not sure is whether Gazelle can generate py_binary without generating py_library like @arrdem said.

@dougthor42dougthor42 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I am not sure is whether Gazelle can generate py_binary without generating py_library.

In general, yes this is possible. Given

# foo.pyif__name__=="__main__":
print("hey")

Then, assuming gazelle:python_generation_mode file, Gazelle will generate:

py_binary(
name="foo",
srcs= ["foo.py"],
)

However, package and project generation modes are different and TBH I'm less familiar with them.

Comment threadCHANGELOG.md Outdated
multiple times.
* (tools/wheelmaker.py) Extras are now preserved in Requires-Dist metadata when using requires_file
to specify the requirements.
* (gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcs: https://github.com/bazel-contrib/rules_python/pull/2822

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrap at ~80chars please

Comment threadgazelle/python/resolve.go Outdated
}
pyLibrarySrcs := otherRule.AttrStrings("srcs")
for _, src := range pyLibrarySrcs {
if src == pyBinarySrcs[0] {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A py_binary can have multiple srcs so you don't want to check against only the 1st value.

What are the requirements for not indexing the binary?

  • Any binary source file is found in any py_library srcs?
  • All binary srcs are found in a single py_library? Eg binary.srcs is a subset of library.srcs.
  • All binary srcs are found in multiple py_library targets?

@linzhp

Copy link
Copy Markdown
Contributor

Should we make Gazelle always generate py_library and stop indexing any py_binary? This is more consistent with other languages.

@yushan26

Copy link
Copy Markdown
ContributorAuthor

Should we make Gazelle always generate py_library and stop indexing any py_binary? This is more consistent with other languages.

Yea that's probably more consistent, although this may be unexpected behavior to the existing gazelle extension.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@yushan26@arrdem@linzhp@dougthor42@yushan8
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); fix(gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcs by yushan26 · Pull Request #2822 · bazel-contrib/rules_python · GitHub
Skip to content

fix(gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcs - #2822

Closed
yushan26 wants to merge 5 commits into
bazel-contrib:mainfrom
yushan26:index-py-library
Closed

fix(gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcs#2822
yushan26 wants to merge 5 commits into
bazel-contrib:mainfrom
yushan26:index-py-library

Conversation

@yushan26

@yushan26yushan26 commented Apr 24, 2025

Copy link
Copy Markdown
Contributor

This PR ensures that py_binary rules are not indexed into Gazelle's IndexMap when there is a corresponding py_library rule with the same srcs. When both py_library and py_binary targets share the same file in their srcs, Gazelle previously indexed both under the same import path. This led to ambiguity and resolution errors, as Gazelle found multiple rules for the same language (py).

To resolve this, the PR updates Gazelle to skip indexing py_binary rules, allowing py_library to be the only import being indexed.

Testing
An integration test was added where both a py_library and a py_binary rule include the same script.py in their srcs. The test verifies that Gazelle correctly resolves the import to the py_library target.

@arrdem

arrdem commented May 7, 2025

Copy link
Copy Markdown
Contributor

I don't think that just not indexing py_binary sources is a workable solution. A pattern we've seen before is that users will want to consider sources part of a py_binary and explicitly not want to manage what's perceived to be a duplicate py_library target, instead expecting other use sites to depend on that fileset via the py_binary target.

This has obvious downsides in that the entrypoint script gets materialized among other things, but it's an existing and working use-case.

I think the behavior in this case should simply be to prefer the library target in case both are an option, not ignoring binaries entirely.

@linzhp

Copy link
Copy Markdown
Contributor

A pattern we've seen before is that users will want to consider sources part of a py_binary and explicitly not want to manage what's perceived to be a duplicate

Are you talking about the scenarios when py_binary is generated but not py_library?

@yushan26yushan26 changed the title Skip indexing py_binary rulesSkip indexing py_binary rules if a corresponding py_library rule contains the same srcsMay 8, 2025
@yushan26

Copy link
Copy Markdown
ContributorAuthor

Gotcha, I updated the logic to skip indexing the py_binary rule only if there is a corresponding py_library rule that also has the samesrcs populated. Let me know if this makes sense.

@yushan26yushan26 changed the title Skip indexing py_binary rules if a corresponding py_library rule contains the same srcsfix(gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcsMay 27, 2025
@yushan26
yushan26 requested a review from rickeylev as a code ownerMay 27, 2025 20:39
@linzhp

Copy link
Copy Markdown
Contributor

@dougthor42 can you also take a look at this PR?

One thing I am not sure is whether Gazelle can generate py_binary without generating py_library like @arrdem said.

@dougthor42dougthor42 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I am not sure is whether Gazelle can generate py_binary without generating py_library.

In general, yes this is possible. Given

# foo.pyif__name__=="__main__":
print("hey")

Then, assuming gazelle:python_generation_mode file, Gazelle will generate:

py_binary(
name="foo",
srcs= ["foo.py"],
)

However, package and project generation modes are different and TBH I'm less familiar with them.

Comment threadCHANGELOG.md Outdated
multiple times.
* (tools/wheelmaker.py) Extras are now preserved in Requires-Dist metadata when using requires_file
to specify the requirements.
* (gazelle): Skip indexing py_binary rules if a corresponding py_library rule contains the same srcs: https://github.com/bazel-contrib/rules_python/pull/2822

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrap at ~80chars please

Comment threadgazelle/python/resolve.go Outdated
}
pyLibrarySrcs := otherRule.AttrStrings("srcs")
for _, src := range pyLibrarySrcs {
if src == pyBinarySrcs[0] {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A py_binary can have multiple srcs so you don't want to check against only the 1st value.

What are the requirements for not indexing the binary?

  • Any binary source file is found in any py_library srcs?
  • All binary srcs are found in a single py_library? Eg binary.srcs is a subset of library.srcs.
  • All binary srcs are found in multiple py_library targets?

@linzhp

Copy link
Copy Markdown
Contributor

Should we make Gazelle always generate py_library and stop indexing any py_binary? This is more consistent with other languages.

@yushan26

Copy link
Copy Markdown
ContributorAuthor

Should we make Gazelle always generate py_library and stop indexing any py_binary? This is more consistent with other languages.

Yea that's probably more consistent, although this may be unexpected behavior to the existing gazelle extension.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@yushan26@arrdem@linzhp@dougthor42@yushan8