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

Please consider a feature flag for URL support, to substantially reduce dependencies #1677

Open
joshtriplett opened this issue Nov 14, 2024 · 3 comments
Labels
acknowledged an issue is accepted as shortcoming to be fixed

Comments

@joshtriplett
Copy link
Contributor

Current versions of gix-url have added quite a few dependencies, including many ICU crates and other dependencies that look related to international domain names.

Please consider adding a feature flag to drop all support for URLs and anything that depends on URLs (other than opaquely), so that software working exclusively with local repositories can avoid all of the dependencies of gix-url.

@Byron Byron added the acknowledged an issue is accepted as shortcoming to be fixed label Nov 15, 2024
@Byron
Copy link
Member

Byron commented Nov 15, 2024

Thanks for the suggestion!

I think it will be helpful with this that the url crate is an implementation detail, and is used once to parse a URL into its constituents.

In theory, gitoxide could adopt the URL parsing routing of Git which parses the URL itself entirely. Whether correct or not, at least that should bring gitoxide closer to Git - the baseline tests currently have a 60% failure rate, so most of the 265 permutations in URLs don't parse similarly.

Given that, I think it will be preferable to push for full compliance and remove the url crate entirely.

@joshtriplett
Copy link
Contributor Author

Whether you do the URL parsing directly or use a crate for it, I would expect that the handling of IDNs and similar will still involve some additional dependencies and overhead. So either way, it may make sense to have a feature flag (enabled by default) controlling URL functionality.

@Byron
Copy link
Member

Byron commented Nov 15, 2024

The key of what gix-url is doing is (probably mostly) implemented in parse_connect_url() and that doesn't know about IDN at all. For compatibility, dealing with it certainly wouldn't be required. I also checked other bits like url.c and urlmatch.c, and they didn't refer to external resources in an obvious way at least.

Being en-par with Git seems like the way to go, also to finally pass all baseline tests, and it should be alright not to deal with IDNs until Git does. And then, I'd think support for IDNs should still be feature-toggled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
acknowledged an issue is accepted as shortcoming to be fixed
Projects
None yet
Development

No branches or pull requests

2 participants