In master/v2, cannot checkout PR base sha

[mydea]

I guess this is related to #15 - I used to be able to get the base commit of a PR (to compare code coverage - here: https://github.com/mydea/ember-cli-code-coverage-action/) and check it out - which stopped working in master/v2. After some digging, I figured it was because that commit was not fetched anymore.

How would I need to configure v2 to make this work again? Thank you!

[ericsciple]

Checkout v2 was optimized for the mainline case, fetch only the single commit.

If it's just the base commit that you need, you can fetch that single commit:

git fetch -c protocol.version=2 fetch --no-tags --progress --no-recurse-submodules --depth=1 origin ${{ github.context.pull_request.base.sha }}

Note, this works because v2-beta leaves the auth token in the git config by default, so scripting authenticated git commands just works.

Thoughts?

I also wonder whether an input to control the refspec would be good. I have some thoughts, but not fully fleshed out yet.

[mydea]

Ah, nice. I'll try it later :) Maybe it would be nice to add things like this to the docs/readme, as I guess it is not an uncommon use case (to compare something for a PR)!

[mansona]

I feel like this would be something to support as part of the default API for this action 🤔 there are plenty of people that will be writing Actions that are trying to do diffs between the head of a PR and master.

something like includeBaseBranch or something might be a reasonable addition

[ericsciple]

Spitballing...

I'm thinking something like an input refspec which allows additional refspecs to be appended to the fetch command.

With some easy options built-in like: branches, tags, all, or totally custom (i.e. is a SHA or contains a :, multiline for multiple)

• if branches then the refspec refs/heads/*:refs/remotes/origin/heads/* is added
• if tags then refs/tags/*:refs/tags/* is added
• if all then branches + tags

maybe another option base ? only works for pull requests, i.e. <BASE_SHA>:<BASE_REF>

[mansona]

Yea that works for me 👍

@ericsciple how do you feel about branches being the default in this context (or maybe base)? If we do base as the default then it can just ignore if we're not on a PR

[ericsciple]

@mansona I think that's fair. The goal of checkout v2 was perf improvements, so we switched to fetching a single commit by default. For a PR I don't think fetching two commits by default is bad (most git objects likely shared by the two commits anyway).

Good discussion here. Thanks.

[ericsciple]

@chrispat fyi, good feedback here to consider re v2 enhancements

[ericsciple]

Here is a similar issue that is related to this discussion.

Although it is a tags scenario. Whereas this issue is about base ref scenario.

[chrispat]

I like the idea of a refspec input that can be used to augment what we fetch. However, I do think it is totally reasonable for us to augment our default refspec in the PR case to include the base, merge and head refs.

[lmsurpre]

Just wanted to mention that GH sets github.base_ref and so, while waiting for this, I think it works to do something like this to get just the target branch (e.g. for diffing):

    - name: Fetch target branch
      env:
        BASE: ${{ github['base_ref'] }}
      run: git fetch --no-tags --prune --depth=1 origin +refs/heads/${BASE}:refs/remotes/origin/${BASE}

Looking forward to removing this from my workflows when this action fetches it by default.

[ericsciple]

Proposal here to add an additional input: #155

[jasonkarns]

In the interim (before fetch-refs support arrives), it seems like adding another "common scenario" to the readme would be helpful.

Opened #213 to add an example scenario inspired by @lmsurpre 's comment above (and my own similar use case)