feat: use git stashes for gitWorkflow #663

iiroj · 2019-07-20T07:44:49Z

This PR rewrites the gitWorkflow to use git stashes instead of cache-busting read-tree and write-tree.

Some benefits:

faster on very large repositories, since the index doesn't get cleared during operations
Since the first task is to create a stash of the original HEAD state, it makes lint-staged more robust to errors, when on failure the stash will either get restored, or in the case lint-staged crashes, left behind for easy manual restore.

This currently passes all current tests (albeit with some modifications since the logic is a bit different), but I'm not sure about situations where there would be merge conflicts between modified and staged, and unstaged changes.

Consider this an evolution of #633, which did not pass all intergration scenarios.

TODO

make a test for a merge conflict situation, where restoring unstaged changes might be impossible because of linter modifications to staged changes.
Handle merge conflicts
Handle untracked files

Fixes

codecov · 2019-07-21T08:23:44Z

Codecov Report

Merging #663 into master will increase coverage by 0.32%.
The diff coverage is 100%.

@@           Coverage Diff            @@
##           master   #663      +/-   ##
========================================
+ Coverage   99.67%   100%   +0.32%     
========================================
  Files          11     11              
  Lines         306    278      -28     
  Branches       57     53       -4     
========================================
- Hits          305    278      -27     
+ Misses          1      0       -1

Impacted Files	Coverage Δ
src/gitWorkflow.js	`100% <100%> (+1.53%)`	⬆️
src/runAll.js	`100% <100%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7432469...1382586. Read the comment docs.

codecov · 2019-07-21T08:36:24Z

Codecov Report

Merging #663 into beta will decrease coverage by 0.54%.
The diff coverage is 98.08%.

@@            Coverage Diff             @@
##             beta     #663      +/-   ##
==========================================
- Coverage   99.67%   99.13%   -0.55%     
==========================================
  Files          11       12       +1     
  Lines         309      347      +38     
  Branches       64       64              
==========================================
+ Hits          308      344      +36     
- Misses          1        3       +2

Impacted Files	Coverage Δ
lib/resolveGitDir.js	`100% <ø> (ø)`
lib/generateTasks.js	`100% <ø> (ø)`
lib/getStagedFiles.js	`100% <ø> (ø)`
lib/printErrors.js	`100% <ø> (ø)`
lib/validateConfig.js	`100% <ø> (ø)`
lib/index.js	`100% <ø> (ø)`
lib/file.js	`100% <100%> (ø)`
lib/resolveTaskFn.js	`100% <100%> (ø)`
lib/runAll.js	`100% <100%> (ø)`
lib/execGit.js	`100% <100%> (ø)`
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3f27bc7...2b57db0. Read the comment docs.

iiroj · 2019-07-21T12:31:33Z

Regarding the TODO, it seems this branch doesn't pass the new test introduced in #664, so there's still something to be done about that.

EDIT: this was solved by retrying with a 3-way merge

iiroj · 2019-07-31T05:31:35Z

@okonet Do you have thoughts on this?

okonet · 2019-07-31T10:26:18Z

Didn’t have time to lock yet. Vacation :)

egeriis · 2019-08-15T10:06:29Z

Been craving a solution like this for a long time, good work @iiroj 🙂

okonet · 2019-08-17T05:49:14Z

I’m wondering where all the tests for the git stash are gone. They look deleted in the branch.

iiroj · 2019-08-17T06:48:26Z

@okonet If you mean tests for the partial staging logic, they have been removed as that is no longer done, but rather stashing is done every time. Since the workflow is different, the old tests no longer apply.

okonet · 2019-08-17T06:50:43Z

Yes, but old tests could be modified since they test merging behavior. Right now there is no test asserting that the merge succeed and the right state being committed.

okonet · 2019-08-17T06:51:37Z

And why is the logic would not apply? It’s still the multi step process so simulating it should give you similar results for git status. What am I missing?

iiroj · 2019-08-17T07:05:27Z

Alright, so the tests for calling of the separate methods have been removed, because they tested only the old logic. The full integration tests are still there and remain the same, as they test commits and git status.

I guess I could rewrite those tests, but they do not increase coverage because runAll.unmocked.spec already covers it.

Looking at runAll, the logic is like this:

Create a backup stash and hide unstaged changes
Run tasks on staged changes
If there are errors, reset and restore everything from backup stash
if there are no errors, apply unstaged changes back
Remove the backup stash

So there could still be tests for 3 and 4, since they are conditional.

Additionally, if there are errors during 4, we should probably revert to the original status and fail committing, because in that case there's something messed up between staged and unstaged changes, caused by the tasks, and manual review would be the best. This is not currently the behaviour in the branch, since 3-way merging seems to handle them.

iiroj · 2019-08-17T07:49:17Z

I rebased this branch on top of unix-paths (#678), because in any case that should be merged first. With this branch, I'm more interested in releasing it under the next tag, so that it can be tested before merging.

okonet · 2019-08-17T08:56:12Z

with 4 that’s not really how it suppose to work since you almost always going to get conflicts with stash and reformatted files. The original version would apply as many changes as possible and ignore the conflicted hunks. I guess your version will just bail out.

That said, this behavior also causes #565 since it ignores all errors which it shouldn’t probably so we can improve that.

iiroj · 2019-08-17T09:20:11Z

Are you open to considering this as a change in the way lint-staged should work (regarding bailing in case of unresolvable merge)? Currently, lint-staged is approaching being unusable in our large repo because of performance issues with it clearing the git index cache while running. This makes git actions take over a minute, and lint-staged itself like two minutes.

I'm also open to trying different approaches to solve this issue.

I think it might be possible to create a diff and apply in the same way as currently (linter modifications over the original state), but it would take multiple stashes.

Git diff completely ignores new, untracked files, but they can be read/written separately

okonet · 2019-10-31T10:27:31Z

@iiroj I have to force push in order to change the branch. Please be aware and do git reset --hard origin/git-stashes next time instead of git pull

okonet · 2019-10-31T10:46:40Z

Ah seems I misunderstood the idea. We have to merge to beta from this one in order to release. Let me try one more time.

okonet · 2019-10-31T10:47:40Z

Ah fuck I screwed it didn't I?

okonet · 2019-10-31T10:52:08Z

🎉 This PR is included in version 9.5.0-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

okonet · 2019-10-31T10:54:18Z

@iiroj apparently it works as expected now I should have not rebased from master onto beta I guess. Not sure if we should try to recover from that or proceed with smaller PRs that we'll merge into beta manually so it's released.

Another thing I noticed, it won't release as major since there are no commits with BREAKING CHANGE marker inside. See https://github.com/okonet/lint-staged/releases/tag/v9.5.0-beta.1%40beta

iiroj · 2019-10-31T13:34:14Z

@okonet I’ll add a breaking change to the commit when I update the readme about git add.

okonet · 2019-10-31T14:20:20Z

@iiroj what should we do about this branch? Should we try restoring it somehow?

okonet · 2019-10-31T14:22:15Z

Ah I see, branch is in place the the PR is closed. We won't be able to re-open it I guess so you'd have to create a new one and refer to this one for the discussion. Okay?

okonet · 2019-10-31T14:22:53Z

Alternatively we could rebase the branch back on master and see if that would help... Do you want me to try?

iiroj · 2019-10-31T15:10:44Z

@okonet nah it’s ok, I’ll open a new PR for the beta branch.

iiroj force-pushed the git-stashes branch 2 times, most recently from c873e14 to 1382586 Compare July 21, 2019 08:23

iiroj force-pushed the git-stashes branch from 1382586 to 9c14edf Compare July 21, 2019 08:36

iiroj force-pushed the git-stashes branch 5 times, most recently from 4b04d15 to dfb013a Compare July 21, 2019 12:30

iiroj force-pushed the git-stashes branch from dfb013a to 0a94ad2 Compare July 21, 2019 13:41

iiroj mentioned this pull request Jul 21, 2019

lint-staged doesn't restore unstaged changed when it fails #550

Closed

iiroj force-pushed the git-stashes branch 3 times, most recently from 66d1b21 to 61b2c03 Compare July 27, 2019 08:31

iiroj force-pushed the git-stashes branch from 61b2c03 to 070bf07 Compare August 3, 2019 08:39

iiroj force-pushed the git-stashes branch from 070bf07 to 08ba381 Compare August 17, 2019 07:47

iiroj force-pushed the git-stashes branch from 08ba381 to 223f569 Compare August 17, 2019 08:51

iiroj added 14 commits October 31, 2019 11:26

test: do not use tmp for creation of tmp directories

05f53b3

test: add mock console

b591ba8

test: further increase timeout for long test

19e6c11

fix: keep untracked files around by backing them up

d20c5be

Git diff completely ignores new, untracked files, but they can be read/written separately

refactor: improvements based on PR review

0018949

docs: remove bit about next release

e3da610

fix: workaround for stashing deleted files for git < 2.23

50afea0

test: should work when amending previous commit with unstaged changes

35b0616

test: improve coverage of gitWorkflow

de82960

feat: automatically stage task modifications

7d0379d

feat: warn when task contains "git add"

4014f3c

test: add test for supplying object config via node api

473b93b

fix: correctly restore untracked files from backup stash

0111f48

fix: prevent Listr from hiding git add warning

2b57db0

okonet force-pushed the git-stashes branch from 0400e92 to 2b57db0 Compare October 31, 2019 10:27

okonet merged commit 2b57db0 into beta Oct 31, 2019

okonet added the released on @beta label Oct 31, 2019

iiroj mentioned this pull request Nov 3, 2019

Use git stashes in git workflow for better performance #724

Merged

iiroj deleted the git-stashes branch November 14, 2019 08:58

chelshaw mentioned this pull request Apr 6, 2020

Ui/update lint staged hashicorp/vault#8686

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: use git stashes for gitWorkflow #663

feat: use git stashes for gitWorkflow #663

iiroj commented Jul 20, 2019 •

edited

codecov bot commented Jul 21, 2019

codecov bot commented Jul 21, 2019 •

edited

iiroj commented Jul 21, 2019 •

edited

iiroj commented Jul 31, 2019

okonet commented Jul 31, 2019

egeriis commented Aug 15, 2019

okonet commented Aug 17, 2019

iiroj commented Aug 17, 2019

okonet commented Aug 17, 2019

okonet commented Aug 17, 2019

iiroj commented Aug 17, 2019 •

edited

iiroj commented Aug 17, 2019

okonet commented Aug 17, 2019 •

edited

iiroj commented Aug 17, 2019

okonet commented Oct 31, 2019

okonet commented Oct 31, 2019

okonet commented Oct 31, 2019

okonet commented Oct 31, 2019

okonet commented Oct 31, 2019

iiroj commented Oct 31, 2019

okonet commented Oct 31, 2019

okonet commented Oct 31, 2019

okonet commented Oct 31, 2019

iiroj commented Oct 31, 2019

feat: use git stashes for gitWorkflow #663

feat: use git stashes for gitWorkflow #663

Conversation

iiroj commented Jul 20, 2019 • edited

TODO

Fixes

codecov bot commented Jul 21, 2019

Codecov Report

codecov bot commented Jul 21, 2019 • edited

Codecov Report

iiroj commented Jul 21, 2019 • edited

iiroj commented Jul 31, 2019

okonet commented Jul 31, 2019

egeriis commented Aug 15, 2019

okonet commented Aug 17, 2019

iiroj commented Aug 17, 2019

okonet commented Aug 17, 2019

okonet commented Aug 17, 2019

iiroj commented Aug 17, 2019 • edited

iiroj commented Aug 17, 2019

okonet commented Aug 17, 2019 • edited

iiroj commented Aug 17, 2019

okonet commented Oct 31, 2019

okonet commented Oct 31, 2019

okonet commented Oct 31, 2019

okonet commented Oct 31, 2019

okonet commented Oct 31, 2019

iiroj commented Oct 31, 2019

okonet commented Oct 31, 2019

okonet commented Oct 31, 2019

okonet commented Oct 31, 2019

iiroj commented Oct 31, 2019

iiroj commented Jul 20, 2019 •

edited

codecov bot commented Jul 21, 2019 •

edited

iiroj commented Jul 21, 2019 •

edited

iiroj commented Aug 17, 2019 •

edited

okonet commented Aug 17, 2019 •

edited