Skip to content

Use git diff in temp repository instead of diff over directories#144

Merged
mrnugget merged 4 commits intomasterfrom
git-diff
Feb 28, 2020
Merged

Use git diff in temp repository instead of diff over directories#144
mrnugget merged 4 commits intomasterfrom
git-diff

Conversation

@mrnugget
Copy link
Copy Markdown
Contributor

This fixes #137.

Before this change src actions exec produced patches by doing the
following:

  1. Download ZIP archive of repository
  2. Unzip archive into directory A
  3. Run action in directory A
  4. Unzip archive into directory B
  5. Run diff A B
  6. Strip 'A' and 'B' prefixes from produced patches

The problem with this approach was that it didn't respect the .gitignore
file in a repository.

And since we run diff outside the folder it's non-trivial to make it
respect the .gitignore, which contains pattern with the assumption that
the root directory is the repository.

This change here uses the following approach:

  1. Download ZIP archive of repository
  2. Unzip archive into directory A
  3. Run git init && git add --all --force & git commit -am "init" in directory
  4. Run action in directory A
  5. Run git add --all && git diff --cached in directory A

That gives us a proper diff produced by git, respecting .gitignore.

There's a cost involved to making a commit with all the files (since the
git objects have to be written), but from manual testing I can say that
it's not noticably slower than unzipping the archive a second time, as
we previously did.

It also gets us rid of the dependency on diff, of which users have a
ton of different versions installed.

Before this change `src actions exec` produced patches by doing the
following:

1. Download ZIP archive of repository
2. Unzip archive into directory A
3. Run action in directory A
4. Unzip archive into directory B
5. Run `diff A B`
6. Strip 'A' and 'B' prefixes from produced patches

The problem with this approach was that it didn't respect the .gitignore
file in a repository.

And since we run diff _outside_ the folder it's non-trivial to make it
respect the .gitignore, which contains pattern with the assumption that
the root directory is the repository.

This change here uses the following approach:

1. Download ZIP archive of repository
2. Unzip archive into directory A
3. Run `git init && git add --all --force & git commit -am "init"` in directory
4. Run action in directory A
5. Run `git add --all && git diff --cached` in directory A

That gives us a proper diff produced by git, respecting .gitignore.

There's a cost involved to making a commit with all the files (since the
git objects have to be written), but from manual testing I can say that
it's not noticably slower than unzipping the archive a second time, as
we previously did.

It also gets us rid of the dependency on `diff`, of which users have a
ton of different versions installed.
Copy link
Copy Markdown
Member

@eseliger eseliger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome improvement!

@mrnugget mrnugget merged commit c423c94 into master Feb 28, 2020
@mrnugget mrnugget deleted the git-diff branch February 28, 2020 10:19
scjohns pushed a commit that referenced this pull request Apr 24, 2023
* Use git diff in temp repository instead of diff over directories

Before this change `src actions exec` produced patches by doing the
following:

1. Download ZIP archive of repository
2. Unzip archive into directory A
3. Run action in directory A
4. Unzip archive into directory B
5. Run `diff A B`
6. Strip 'A' and 'B' prefixes from produced patches

The problem with this approach was that it didn't respect the .gitignore
file in a repository.

And since we run diff _outside_ the folder it's non-trivial to make it
respect the .gitignore, which contains pattern with the assumption that
the root directory is the repository.

This change here uses the following approach:

1. Download ZIP archive of repository
2. Unzip archive into directory A
3. Run `git init && git add --all --force & git commit -am "init"` in directory
4. Run action in directory A
5. Run `git add --all && git diff --cached` in directory A

That gives us a proper diff produced by git, respecting .gitignore.

There's a cost involved to making a commit with all the files (since the
git objects have to be written), but from manual testing I can say that
it's not noticably slower than unzipping the archive a second time, as
we previously did.

It also gets us rid of the dependency on `diff`, of which users have a
ton of different versions installed.

* Remove mention of `zip` and `diff` from README

* Check that git is available in src actions exec

* Add note about git requirement to README
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.

Diff produced by src actions exec doesn't respect gitignore

4 participants