Add support for different LFS url to vcpkg_from_git#28693
Conversation
|
The tests are failing on Windows because the agent image has an old version of Git LFS: 2.13.3 which is almost two years out of date. The test script needs the I'm not sure how to fix this other than updating the VM images, which I don't know how to do. |
@vicroms, could you please take a look? |
|
@BillyONeal would it be possible to update Git LFS on our VMs? |
We are deploying the copy that comes with Visual Studio. If the user needs a copy more recent than that I have serious concerns about being able to accept that in a port. |
|
@BillyONeal This is just for testing. The scripts don't need anything more recent normally, but I can't implement a proper test without some newer LFS features. I could just disable that part of the test if the LFS version is too old, but then it's not automatically tested... |
We don't have a means of restricting such a machine-wide configuration change to your test. |
|
@BillyONeal I'm surprised that the vcpkg VM images are even using a two year old Git LFS version... The default Azure hosted agent images are up-to-date. |
That's what comes with Visual Studio 2022.
The default Azure hosted images contain everything under the sun because they are trying to maximize the likelihood of your build working.
Intentionally? Probably not. But we invoke arbitrary user build scripts which could.
Or we could just wait until a version which does what you need has enough market penetration that we can confidently assume its presence. |
|
@BillyONeal How does vcpkg even handle testing ports that require specific tooling if it can't be installed? I know that vcpkg acquires some programs automatically, but looking at the code it just asks you to install it yourself in many cases. Maybe the oldest viable LFS version (3.0.0 from March 2021) can be installed instead of the latest one? But if you insist on waiting for LFS to be updated externally, then feel free to close this PR. I don't personally need this change, I just did it because someone asked for it and I thought it would be a quick and easy. I don't want to spend that much more time working on it. |
It depends. But usually these days we probably wouldn't accept adding stuff like that to the curated registry. Ports are on the hook for being broadly installable with a "typical developer workstation". Seeing that we have something only for it to explode in your face is a bad experience. The one big notable exception I'm aware of right now is CUDA, but pretty much every port that wants CUDA has that controlled by an optional feature. (And there, it isn't that we wouldn't solve the problem of getting what's necessary, it's that we are legally unable to under NVIDIA's license terms)
I will confirm with other maintainers before going there. (Should know by ~Thursday) |
|
@vicroms @dan-shaw @ras0219-msft @AugP @JavierMatosD and I discussed this today and came up with the following outcomes:
Does that sound reasonable @DDoS ? |
|
@BillyONeal Sounds good. I'm away for a week, so I'll make this change next Thursday or Friday. |
8ec6a24 to
5f8c2ee
Compare
|
I will merge this closer to the end of the day. (Just don't want to push a merge conflict with the world while still processing today's changes) |
|
Thank you! |
Add an optional argument to the
LFSkeyword invcpkg_from_git, so a different URL can be used. Update the documentation and tests.Fixes #28593