-
Notifications
You must be signed in to change notification settings - Fork 9
Sanitize dependency names for filesystem compatibility #289
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
Conversation
Switches the replacement character from underscore to hyphen when sanitizing dependency names for directory creation, ensuring consistency with wit-deps naming conventions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Just one small concern I have is the choice of the separator - I forget, is - a valid character to use in IDs (could you check the spec and ideally add a comment with a reference?)
If - is accepted, could we use some other character, which is disallowed by component model, but would be Windows-path compatible?
_ comes to mind, but I do not have the time to check the spec right now
Clarified comments in the dependency name sanitization process to specify the requirements for WIT identifiers, including kebab-case formatting and compatibility with Windows file systems.
| // WIT identifiers must be kebab-case (words separated by '-') and cannot contain underscores or other punctuation. | ||
| // See: https://component-model.bytecodealliance.org/design/wit.html#identifiers | ||
| // We use '-' as the separator to match WIT spec and ensure Windows compatibility. | ||
| let sanitized_id = id.replace(':', "-"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this would cause a clash between foo:bar-baz and foo-bar:baz, right?
From the linked spec:
Double hyphens (--) are not allowed.
Can we try using -- to work around that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
again you meant double hyphens?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, anything that would be disallowed by the WIT spec and allowed by most OSes, Windows in particular
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double hyphens (--) are not allowed.
didn't this mean continuous double hyphens?
foo-bar-baz allowed but not foo--bar?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, so I'm thinking if we stored:
foo:bar-bazpackage ->foo--bar-bazdirectoryfoo-bar:bazpackage ->foo-bar--bazdirectory
Tooling like wit-bindgen would be able to read WIT from these directories and rely on WIT package name (package directive in WIT).
I don't think any component model tooling actually relies on directory names for anything
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
foo--bar-baz is not allowed because it contains -- (continuous double hyphens)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exactly, that's why we should be able to use that on the file system to avoid duplicates - no WIT package name will ever contain --
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this would cause a clash between foo:bar-baz and foo-bar:baz, right?
Theoretically yes but practically?
Can we try using
--to work around that?
Did you mean em-dash (—)?
|
@SmartManoj given that upstream usage has been fixed, is this still relevant? |
Ensure dependency names are sanitized by replacing colons with underscores to maintain filesystem compatibility.
Fixes #285