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

envbuilder: fetch upstream changes from repo if local copy is not dirty #281

Open
reiser opened this issue Jul 24, 2024 · 10 comments
Open
Assignees

Comments

@reiser
Copy link

reiser commented Jul 24, 2024

Is it possible to disable the /workspace cache of a coder build with envbuilder.
Even after restart work spaces it caches to old commit

#1: 📦 Cloning https://bitbucket.org/lorem/repo.git to /workspaces/ipsum..
#1: 👤 Using no authentication!
#1: 📦 The repository already exists! [672.333µs]
@johnstcn
Copy link
Member

johnstcn commented Aug 3, 2024

What should envbuilder do in this case if the repository contains uncommitted changes?
Should it still fetch changes from the remote?

@megla-tlanghorst
Copy link

megla-tlanghorst commented Aug 7, 2024

Couldn't it check for local changes? I had this when the devcontainer build failed and I fixed it in the repo, but I had to destroy the workspace because I couldn't update it easily obviously.

Basically just:

if (repo has no changes {
  git pull
}

Maybe even just as an option

@johnstcn johnstcn changed the title envbuilder caches repo envbuilder: fetch upstream changes from repo if local copy is not dirty Aug 7, 2024
@mafredri
Copy link
Member

mafredri commented Aug 8, 2024

@megla-tlanghorst would the --remote-repo-build-mode option we recently introduced cover your use-case?

Automatically updating the users repo is an interesting proposition too, though. There are some caveats though:

  • What do we consider dirty? Uncommitted changes? Untracked files?
  • What do we do if there's an error? I.e. untracked file -> pull -> conflict
  • What if the user was working on a specific feature but since we pulled the changes they're no longer sure where they were (if it's an opt-in option, the onus is on the user, but it's still a potentially bad experience)

@megla-tlanghorst
Copy link

@megla-tlanghorst would the --remote-repo-build-mode option we recently introduced cover your use-case?

Maybe, that's what I'm currently testing.

@johnstcn
Copy link
Member

johnstcn commented Aug 8, 2024

Alternative possibility is add -> stash -> pull -> unstash. This does run the risk of users being left to clean up a nasty merge conflict though.

@johnstcn
Copy link
Member

johnstcn commented Sep 13, 2024

Parking this for now due to some open questions:

  • Do we fetch and checkout versus pull --ff-only? The former seems safer to me. Or do we stash -> pull -> unstash?
  • What do we do in case of local changes? It seems reasonable to just skip fetching updates completely if the worktree is dirty, but is this the behaviour that users expect?
  • What should we do if a user attempts to clone a different Git repo into an existing one? Right now, we just skip cloning altogether if we detect an existing repo. Should we update the existing repo, or bail entirely?
  • What if the pre-existing repo has a different remote configured? Which remote do we use? The same as ENVBUILDER_GIT_URL?

@mafredri
Copy link
Member

mafredri commented Sep 13, 2024

@johnstcn I think we should only operate on a clean repository with the default/configured branch checked out. If a user has local changes or a custom branch checked out then we must assume they were in the middle of doing something, and we can't risk removing their anchor, or worse, introduce merge conflicts.

We can instead add a destructive option which simply overwrites the whole repository (in its simplest form: delete + new checkout). This is for anyone who wants the workspaces to be ephemeral, but still uses a permanent storage for some reason.

@johnstcn
Copy link
Member

@mafredri One other scenario I could imagine is that a different git repo is cloned to the path into which we wish to clone. Do you think it's sufficient to check that the URL of the remote "origin" is the same as ENVBUILDER_GIT_URL (after parsing / normalization)?

@mafredri
Copy link
Member

mafredri commented Sep 13, 2024

If I'm understanding you correct, I'd say yeah we want to do that. Essentially we want to check that the remote of the currently checked out branch matches that of envbuilder git URL (name, like "origin" shouldn't matter). If the envbuilder git URL specifies a branch, the currently checked out branch should match as well (match by the remote tracking branch, not necessarily local name). Assuming the repo is clean1 too, we can update.

1 Clean repo means no changes to tracked files and no staged changes. Untracked files should be safe to ignore as I think users wouldn't want those to block the update in most cases.

@mtojek
Copy link
Member

mtojek commented Nov 4, 2024

Co-assigning @SasSwart to continue/take over the WIP PR.

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

No branches or pull requests

7 participants