dagger / dagger-for-github

GitHub Action for Dagger
https://github.com/marketplace/actions/dagger-for-github
Apache License 2.0
120 stars 25 forks source link

build and install dagger from source #53

Closed crazy-max closed 2 years ago

crazy-max commented 2 years ago

fixes #35

Signed-off-by: CrazyMax crazy-max@users.noreply.github.com

shykes commented 2 years ago

Could you explain what this does? Does it build dagger from source, or does it rely on a binary-as-a-service on the upstream dagger side? Thanks!

marcosnils commented 2 years ago

It's building Dagger from the version argument using the Dockerfile and then extracting the binary to cache it in GA

crazy-max commented 2 years ago

Could you explain what this does? Does it build dagger from source, or does it rely on a binary-as-a-service on the upstream dagger side? Thanks!

Yes like @marcosnils says it builds Dagger on-fly using a remote Git context through version input that can be any repository (upstream or fork) that contains the Dockerfile to build Dagger, then extracts built binary in the final stage to the client, cache the file to the hosted tool cache on the GitHub Runner using the official GitHub toolkit. Cache key is the commit sha:

jobs:
  deploy:
    runs-on: ubuntu-latest
    steps:
      -
        name: Checkout
        uses: actions/checkout@v3
      -
        name: Dagger
        uses: dagger/dagger-for-github@v2
        with:
          # build PR 2288
          version: https://github.com/dagger/dagger.git#refs/pull/2288/head
          cmds: do test
jobs:
  deploy:
    runs-on: ubuntu-latest
    steps:
      -
        name: Checkout
        uses: actions/checkout@v3
      -
        name: Dagger
        uses: dagger/dagger-for-github@v2
        with:
          # build main from shykes/dagger
          version: https://github.com/shykes/dagger.git#main
          cmds: do test
shykes commented 2 years ago

I don’t want to hold up this feature, so consider this advisory feedback only. It’s not my preference to decentralize the build process, because it introduces fragmentation and is one step towards loss of control of the developer experience. For example, once we merge this, we have just agreed to am implicit API: we must always have a Dockerfile at the root of the dagger repo, and it must follow the following convention: (whatever this code expects). If we break this convention in the future, we break our user’s github actions. This is not great.

One instance is not the end of the world (which is why this is advisory only, also because I don’t like to be the bad guy saying no all the time, I’ve done it for many years and it’s a thankless job). But if this sets an example and every downstream integration starts building their own version… The problem will get exponentially worse. So I’m more worried about the precedent.

This is why I proposed a “binary on demand” service, it allows us to solve this problem once for all platforms, not just Github.

In summary:

  1. Not my preference to do it this way
  2. Ok to merge this anyway
  3. Please no more downstream builds, next time let’s ask for contribution of a binary-on-demand builder instead, that solves it for everyone.
marcosnils commented 2 years ago

If we break this convention in the future, we break our user’s github actions. This is not great.

This is partially true, correct? Since it will only break for users that have specified @master in their version URL where the Dockerfile doesn't exist at the same location as it was before.

My second thought here is that this specific use-case of the version field I have the impression that it will mostly be used by advanced users that require the bleeding edge Dagger features which haven't been released yet. If we keep a relatively short release cycle, this will have an ever more diminished impact.

This is why I proposed a “binary on demand” service, it allows us to solve this problem once for all platforms

It'd be great to have something like gobinaries.com (https://github.com/tj/gobinaries) that we can leverage only for Dagger binaries. I think that right now might not be the moment to allocate efforts into hosting and setting up such service.

Moving forward I see three possible options:

1 - Merge this "as-is" and live with the possible fragmentation in the short term until we work on either 2 or 3 2 - Setup a binary publishing process into dl.dagger.io for every commit into master and opened PR's 3 - Setup something like gobinaries.com so we can build binaries on the fly for the target platform.

I'd go for 1 for the moment since I personally don't see the fragmentation problem a big issue given the case that it will mostly "potentially" affect a very small subset of advanced users.

shykes commented 2 years ago

If we break this convention in the future, we break our user’s github actions. This is not great.

This is partially true, correct? Since it will only break for users that have specified @master in their version URL where the Dockerfile doesn't exist at the same location as it was before.

Correct it won’t break all our users all at once, only some users some of the time.

My second thought here is that this specific use-case of the version field I have the impression that it will mostly be used by advanced users that require the bleeding edge Dagger features which haven't been released yet. If we keep a relatively short release cycle, this will have an ever more diminished impact.

Yes probably true.

It'd be great to have something like gobinaries.com (https://github.com/tj/gobinaries) that we can leverage only for Dagger binaries. I think that right now might not be the moment to allocate efforts into hosting and setting up such service.

Also probably true.

Moving forward I see three possible options:

1 - Merge this "as-is" and live with the possible fragmentation in the short term until we work on either 2 or 3 2 - Setup a binary publishing process into dl.dagger.io for every commit into master and opened PR's 3 - Setup something like gobinaries.com so we can build binaries on the fly for the target platform.

I'd go for 1 for the moment since I personally don't see the fragmentation problem a big issue given the case that it will mostly "potentially" affect a very small subset of advanced users.

Yes option 1 is fine. Fragmentation issues always seem fine in the beginning, it’s a death by a thousand cuts. That’s why it almost always happens. The people warning against it can always be told “come on this instance is not that bad, aren’t you overreacting?”. It’s true this time, and will also be true the next 10,000 times.

marcosnils commented 2 years ago

As a final reference I was checking how the terraform action versioning worked and they have an option to specify both rc and beta releases here https://github.com/hashicorp/setup-terraform#inputs. I guess the next step would be to follow a similar approach from the Dagger side where we publish snapshot binaries each time a merge is done into main. This will make this source build version feature pretty much a very specialized case.

crazy-max commented 2 years ago

@shykes @marcosnils Thanks for your feedback!

I agree with you on the drifting part if changes occur upstream and break users workflow. Even if we take care of making it clear to be careful on changes in the upstream Dockerfile, user might still want to make some changes in it that would anyway break his workflow. So yeah chicken and egg situation there.

1 is still valid but 2 is more resilient I agree. We try that in the past in buildx repo by publishing the artifacts in the workflow space using https://github.com/actions/upload-artifact/#where-does-the-upload-go to share them and then use the GitHub API to retrieve the artifact in the action but artifacts were not available publicly :disappointed:. This might have changed though :thinking:

morlay commented 2 years ago

For me. Building from source is important.

Not all PRs could be merge into main asap.

Here are two important features for my usage:

So I have to create release branch with them, with this PR's help, I could start to migrate to dagger.