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

fix for case where a branch has first been cloned and then the branch name changes #46

Merged

Conversation

davetroiano
Copy link
Contributor

Bug repro that this fixes:

  • build / run site once with branch branchA specified in plugin config:
options: {
   name: 'repo-my-repo',
   remote: <REMOTE>,
   branch: 'branchA'
 }
  • stop site, change branchA to branchB, run again
  • observe that the locally cached repo doesn't fully go away so we fall into the code block of this fix and hit this error that the remote add / fetch / checkout fix avoids: Error: fatal: ambiguous argument 'origin/branchB': unknown revision or path not in the working tree.

@darthsteven
Copy link
Collaborator

I can reproduce this error with Gatsby 5, but not Gatsby 2...so looks like this bug has snuck in somewhere.
We're still getting the message about deleting the cache because the config changed, but then as you say, the cache isn't actually deleted. Weird.

@darthsteven
Copy link
Collaborator

Yeah, looks like we're essentially doing our own caching thing, rather than using the in-built API for caching (which probably didn't exist when this source plugin was first written).

I think that gatsby no-longer deletes the entire .cache folder, but tries to be smarter about what to invalidate.

Some thoughts:

  • We could keep our own cache system, but hash the plugin options and use that as a subdirectory.
  • We could leverage the gatsby cache system to store a unique key and store all our items in there, which is kinda option 1 tbh.
  • I don't exactly know where gatsby keeps it's cache, but maybe there's an API to hook into and get gatsby to remove our files.
  • Maybe we need to tweak the location of the stored files, and then the invalidation would happen for us.

@darthsteven
Copy link
Collaborator

As far as I can tell, this is Gatsby's logic for removing the cache directories:

https://github.com/gatsbyjs/gatsby/blob/fd8de341684df7aa5fcd911a25786beac471925c/packages/gatsby/src/services/initialize.ts#L438-L461

And that essentially uses globby which uses fast-glob, which presumably means that it's not removing the .git folders within our cache directories, hence the issues you're seeing.

@darthsteven
Copy link
Collaborator

Okay, I've split that cache clearing issue out to #51, which will solve most of the problems fixed by this code, but won't actually fix the issue if you're using a local clone, so I still think there's value in merging this code too.

I'll review it in that sense in a bit, but from a first glance, it looks good, thanks!

@darthsteven darthsteven merged commit a95a06f into stevetweeddale:master Aug 4, 2023
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

3 participants