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.

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

Ignore packages with empty names #5294

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

claytoncasey01
Copy link

@claytoncasey01 claytoncasey01 commented Mar 7, 2025

If the package has an empty name skip it. It can cause errors downstream later due to creating invalid PUrls that can't be parsed.

Fixes: #5261.

Copy link
Contributor

github-actions bot commented Mar 7, 2025

Test Results

3 480 tests  ±0   3 476 ✅ ±0   1m 54s ⏱️ +18s
  392 suites ±0       4 💤 ±0 
   30 files   ±0       0 ❌ ±0 

Results for commit da14005. ± Comparison against base commit 09b17d6.

@@ -452,6 +452,12 @@ func getPackageFromRegistryKeyItems(children []registry.RegistryKeyItem, platfor
return nil
}

if displayName == "" || displayVersion == "" {
Copy link
Member

Choose a reason for hiding this comment

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

why do we have empty packages in the first place?

Copy link
Author

@claytoncasey01 claytoncasey01 Mar 7, 2025

Choose a reason for hiding this comment

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

From a bit of quick research it seems to be somewhat common (for windows): partial installs, bad uninstalls, or even some software just not putting a DisplayName in the registry during the setup/install process.

I can debug the registry code where we pull display value and then actually check the path that's failing to confirm this.

Copy link
Member

Choose a reason for hiding this comment

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

I just want to find out if we need to fix something in cnquery. We should mark this as TODO, since generally we should not have empty package names in the first place.

Sign up for free to 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.

Ignore packages with emtpy name on Windows
2 participants