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

vpm: optimize performance by adding filter when cloning #21216

Merged
merged 1 commit into from
Apr 7, 2024

Conversation

ttytm
Copy link
Member

@ttytm ttytm commented Apr 7, 2024

No description provided.

Copy link
Member

@spytheman spytheman left a comment

Choose a reason for hiding this comment

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

Good work.

@spytheman spytheman merged commit 3835308 into vlang:master Apr 7, 2024
54 checks passed
@ttytm ttytm deleted the vpm/add-clone-filter branch April 7, 2024 14:14
@Wertzui123
Copy link
Contributor

This breaks VPM on systems with an older Git version as --also-filter-submodules is fairly new.
Maybe we should revert this change or at least check for the Git version before adding the flag to the clone command?

@ttytm
Copy link
Member Author

ttytm commented Apr 11, 2024

Good catch. Thanks for reporting. The version check should be good or removing just this flag. We can keep the filtering benefit then.

@JalonSolov
Copy link
Contributor

Version check would be best. Nobody should be using old versions of Git - if their package manager doesn't have the newer version, they can always clone it and build it themselves.

Too many things can go wrong with older versions of Git. :-\

@Wertzui123
Copy link
Contributor

Version check would be best. Nobody should be using old versions of Git - if their package manager doesn't have the newer version, they can always clone it and build it themselves.

Too many things can go wrong with older versions of Git. :-\

While I agree with you in principle, the "old version" you're referring to is from just 2022. And yes, you could theoretically build Git manually (which I have tried earlier today), but especially if you're not very familiar with C build systems it might be a little (and mostly unnecessary) challenge. Furthermore, I need to use VPM in a Debian 10 CI/CD script (for BC reasons), which is why building Git manually is even more unsuited.

@JalonSolov
Copy link
Contributor

The version check I'm referring to would check the version of Git that's available, and add the extra option if it was "new enough".

As for the age of Git... it doesn't matter. They don't do new versions on a whim... the new versions are to fix problems found in the previous version. Rarely, the new version will have a new feature (such as this new option).

Either way, you're always better off with the latest version of Git, since your entire codebase depends on it working properly.

If Debian made this hard to keep up with, the fault is with Debian.

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.

None yet

4 participants