fix: fixes #2138 stash push/pop issue with correct stash drop and adds regression test (#2150)

* test: add regression test for stash push/pop issue (#2138) * fix: ensure newline at end of reflog entries and correct last stash commit retrieval. fixes #2138 * fix: reverse order of stashes in list and update reflog writing logic

Author: bekatan

.all-contributorsrc

@@ -837,6 +837,16 @@
  "userTesting"
  ]
  },
+ {
+ "login": "bekatan",
+ "name": "Bekatan Satyev",
+ "avatar_url": "https://avatars.githubusercontent.com/u/19550476?v=4",
+ "profile": "https://github.com/bekatan",
+ "contributions": [
+ "code",
+ "test"
+ ]
+ },
  {
  "login": "hemanthkini",
  "name": "Hemanth Kini",

README.md

@@ -391,6 +391,7 @@ Thanks goes to these wonderful people ([emoji key](https://github.com/kentcdodds
  <td align="center"><a href="https://github.com/ARBhosale"><img src="https://avatars.githubusercontent.com/u/26981417?v=4?s=60" width="60px;" alt=""/><br /><sub><b>Aniket Bhosale</b></sub></a><br /><a href="https://github.com/isomorphic-git/isomorphic-git/commits?author=ARBhosale" title="Code">💻</a> <a href="https://github.com/isomorphic-git/isomorphic-git/commits?author=ARBhosale" title="Documentation">📖</a> <a href="https://github.com/isomorphic-git/isomorphic-git/commits?author=ARBhosale" title="Tests">⚠️</a></td>
  <td align="center"><a href="https://github.com/gnillev"><img src="https://avatars.githubusercontent.com/u/8965094?v=4?s=60" width="60px;" alt=""/><br /><sub><b>Mathias Nisted Velling</b></sub></a><br /><a href="https://github.com/isomorphic-git/isomorphic-git/commits?author=gnillev" title="Code">💻</a> <a href="https://github.com/isomorphic-git/isomorphic-git/commits?author=gnillev" title="Tests">⚠️</a></td>
  <td align="center"><a href="https://github.com/acandoo"><img src="https://avatars.githubusercontent.com/u/117209328?v=4?s=60" width="60px;" alt=""/><br /><sub><b>acandoo</b></sub></a><br /><a href="#platform-acandoo" title="Packaging/porting to new platform">📦</a> <a href="#userTesting-acandoo" title="User Testing">📓</a></td>
+ <td align="center"><a href="https://github.com/bekatan"><img src="https://avatars.githubusercontent.com/u/19550476?v=4?s=60" width="60px;" alt=""/><br /><sub><b>Bekatan Satyev</b></sub></a><br /><a href="https://github.com/isomorphic-git/isomorphic-git/commits?author=bekatan" title="Code">💻</a> <a href="https://github.com/isomorphic-git/isomorphic-git/commits?author=bekatan" title="Tests">⚠️</a></td>
  <td align="center"><a href="https://github.com/hemanthkini"><img src="https://avatars.githubusercontent.com/u/3934055?v=4?s=60" width="60px;" alt=""/><br /><sub><b>Hemanth Kini</b></sub></a><br /><a href="https://github.com/isomorphic-git/isomorphic-git/commits?author=hemanthkini" title="Code">💻</a></td>
  </tr>
 </table>

__tests__/test-stash.js

@@ -654,8 +654,8 @@ describe('stash drop', () => {
 
  const stashList = await stash({ fs, dir, gitdir, op: 'list' })
  expect(stashList).toEqual([
- 'stash@{0}: stash one: 3ca31f1 initial commit',
- 'stash@{1}: stash three: 3ca31f1 initial commit',
+ 'stash@{0}: stash three: 3ca31f1 initial commit',
+ 'stash@{1}: stash one: 3ca31f1 initial commit',
  ])
  })
 })
@@ -804,12 +804,72 @@ describe('stash pop', () => {
 
  const stashList = await stash({ fs, dir, gitdir, op: 'list' })
  expect(stashList).toEqual([
- 'stash@{0}: stash one: 3ca31f1 initial commit',
- 'stash@{1}: stash three: 3ca31f1 initial commit',
+ 'stash@{0}: stash three: 3ca31f1 initial commit',
+ 'stash@{1}: stash one: 3ca31f1 initial commit',
  ])
  const aContent = await fs.read(`${dir}/a.txt`)
  expect(aContent.toString()).toEqual(aNewContent) // make sure the 2nd staged changes are applied
  const bContent = await fs.read(`${dir}/b.js`)
  expect(bContent.toString()).toEqual(bNewContent) // make sure the 2nd staged changes are applied
  })
 })
+
+describe('stash regression #2138', () => {
+ it('should not lose stashes after stash pop followed by stash push', async () => {
+ const { fs, dir, gitdir } = await makeFixtureStash('stashRegression')
+ await addUserConfig(fs, dir, gitdir)
+
+ // --- stash 1 ---
+ await fs.write(dir + '/a.txt', 'change 1')
+ await stash({ fs, dir, gitdir, message: 'stash 1', op: 'push' })
+ // --- stash 2 ---
+ await fs.write(dir + '/a.txt', 'change 2')
+ await stash({ fs, dir, gitdir, message: 'stash 2', op: 'push' })
+
+ let stashes = await stash({ fs, dir, gitdir, op: 'list' })
+ expect(stashes.length).toBe(2)
+
+ // Pop stash 2
+ await stash({ fs, dir, gitdir, op: 'pop' })
+
+ stashes = await stash({ fs, dir, gitdir, op: 'list' })
+ expect(stashes.length).toBe(1)
+ // Push stash 2 again
+ await stash({ fs, dir, gitdir, message: 'stash 2', op: 'push' })
+
+ stashes = await stash({ fs, dir, gitdir, op: 'list' })
+ expect(stashes.length).toBe(2)
+ })
+
+ it('stash list order before and after stash drop', async () => {
+ const { fs, dir, gitdir } = await makeFixtureStash('stashRegression')
+ await addUserConfig(fs, dir, gitdir)
+
+ // --- stash 1 ---
+ await fs.write(dir + '/a.txt', 'change 1')
+ await stash({ fs, dir, gitdir, message: 'stash 1', op: 'push' })
+ // --- stash 2 ---
+ await fs.write(dir + '/a.txt', 'change 2')
+ await stash({ fs, dir, gitdir, message: 'stash 2', op: 'push' })
+
+ // --- stash 3 ---
+ await fs.write(dir + '/a.txt', 'change 3')
+ await stash({ fs, dir, gitdir, message: 'stash 3', op: 'push' })
+
+ let stashes = await stash({ fs, dir, gitdir, op: 'list' })
+ expect(stashes).toEqual([
+ 'stash@{0}: stash 3: 3ca31f1 initial commit',
+ 'stash@{1}: stash 2: 3ca31f1 initial commit',
+ 'stash@{2}: stash 1: 3ca31f1 initial commit',
+ ])
+
+ // drop stash 3
+ await stash({ fs, dir, gitdir, op: 'drop' })
+
+ stashes = await stash({ fs, dir, gitdir, op: 'list' })
+ expect(stashes).toEqual([
+ 'stash@{0}: stash 2: 3ca31f1 initial commit',
+ 'stash@{1}: stash 1: 3ca31f1 initial commit',
+ ])
+ })
+})

src/commands/stash.js

@@ -174,7 +174,11 @@ export async function _stashDrop({ fs, dir, gitdir, refIdx = 0 }) {
  const stashReflogPath = stashMgr.refLogsStashPath
  await acquireLock({ reflogEntries, stashReflogPath, stashMgr }, async () => {
  if (reflogEntries.length) {
- await fs.write(stashReflogPath, reflogEntries.join('\n'), 'utf8')
+ await fs.write(
+ stashReflogPath,
+ reflogEntries.reverse().join('\n') + '\n',
+ 'utf8'
+ )
  const lastStashCommit = reflogEntries[reflogEntries.length - 1].split(
  ' '
  )[1]