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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃巺 Fetch before rebuild #1157

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

Conversation

flip111
Copy link
Contributor

@flip111 flip111 commented Dec 25, 2023

@f-f
Copy link
Member

f-f commented Dec 27, 2023

I don't think this is fixing the issue - the dependencies from the build plan are not passed in anywhere, and these are the ones we need to fetch.

A good first step for a fix would be to write a test that replicates the issue in #1110. After we get CI to fail because of that we can then write the correct patch.

@flip111
Copy link
Contributor Author

flip111 commented Dec 29, 2023

I'm not a fan of ttd i rather write the patch first if i go ahead with this. Should Fetch.run called multiple times for each dependency or can i pass all the deps at once?

@f-f
Copy link
Member

f-f commented Dec 29, 2023

This is not about TTD, but rather about replicating the failure in a standard environment (CI). If we can't replicate we can't fix.

@flip111
Copy link
Contributor Author

flip111 commented Dec 31, 2023

Can the registry be queried as to what is the latest version of a package?

@f-f
Copy link
Member

f-f commented Dec 31, 2023

Why would we need that? We'll need to build with whatever versions the solver returns, which is not always going to be the latest one.

@flip111
Copy link
Contributor Author

flip111 commented Dec 31, 2023

As new versions get added to the registry, the solver might resolve to something else. I expect the test to start breaking.

@f-f
Copy link
Member

f-f commented Jan 3, 2024

I expect the test to start breaking.

It's entirely possible to have a test that doesn't break over time - we'll need to set the version bounds to be different than what the package set contains.

@flip111
Copy link
Contributor Author

flip111 commented Jan 3, 2024

In #1110 i don't think it was relevant that an outdated package 0.4.1 was available in the cache. The problem should just reproduce when removing any package from the cache which then should be refetched before rebuild. That the problem occurs due to an upgraded version of a dependency is not relevant to test the solution.

@flip111
Copy link
Contributor Author

flip111 commented Jan 3, 2024

The test passes but i don't think it works correctly. Not sure what is happening

@f-f
Copy link
Member

f-f commented Jan 14, 2024

This has been discussed on Discord, but I'll report here as well:
Spago uses a global cache and I think that's ultimately the reason why the new test is not working - you are deleting the package from the local cache, but Spago will find it in the global cache so it just copies it back there.
The user-facing command to get the location of the global cache is spago ls paths, but in tests you can use the Spago.Paths module

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

2 participants