Skip to content

Conversation

kpenfound
Copy link
Contributor

@kpenfound kpenfound commented Sep 15, 2025

Fixes #164

Check for an existing dagger install, and if its version matches the requested version, skip the installation step

@kpenfound kpenfound requested review from jedevc and matipan September 15, 2025 19:00
@jedevc
Copy link
Member

jedevc commented Sep 16, 2025

SGTM, but I think we could maybe avoid an error if the version if the right version of dagger is already installed?

@kpenfound
Copy link
Contributor Author

kpenfound commented Sep 16, 2025

@jedevc thats the dilemma, right now it doesn't care about the version at all, only whether there is already a dagger binary in the path, and there is no error state. so you have:

preinstalled version requested version result without force-install result with force-install
foo bar foo bar
none bar bar bar
foo foo foo (skipped) foo (downloaded)

my goal was to avoid error states, but we also don't want surprising behavior. The thinking is that if you have dagger pre-installed, you probably expect it to be there. If you want to override that with a specific version, there's a path to do that

Signed-off-by: kpenfound <[email protected]>
@jedevc
Copy link
Member

jedevc commented Sep 18, 2025

I think the thing that confuses me with the table is that you can request bar and end up in a state where you're actually running foo.

IMO, if you set bar, then you should always get bar - that's the behavior on main today (by essentially always forcing the install).

My suggestion would be to detect the version of the currently installed dagger, and check if it's the requested version (mildly annoying for commit builds off of main, but probably still doable). If it's the request version, skip, if not, then either, no particular preference:

  • Overwrite it
  • Error

I think that means that we wouldn't have a force-install option anymore?

@kpenfound
Copy link
Contributor Author

@jedevc yeah we can definitely do that, I saw the script linked in the original issue. What I'm confused about in that case is why this was a blocker for using this action in dagger/dagger @matipan . Is it really about avoiding the ~18mb download?

@kpenfound kpenfound changed the title avoid overwriting existing dagger install unless force-install is true avoid downloading dagger if requested version is already installed Sep 18, 2025
@kpenfound
Copy link
Contributor Author

@jedevc @matipan updated based on our live discussion!

Signed-off-by: kpenfound <[email protected]>
@kpenfound kpenfound merged commit d913e70 into dagger:main Sep 22, 2025
11 checks passed
@kpenfound kpenfound deleted the check_existing_install branch September 22, 2025 16:04
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.

✨ Do not install dagger CLI on runner if already present in PATH unless version: specified and different from installed version

3 participants