Git status reports "unmodified" when an integer in a file is changed

[alex996]

I'm running into an issue whereby if I change a single integer in a file, git.status() reports "unmodified" when it should be "*modified". Here is a minimal repro:

https://github.com/alex996/git-int-test/blob/master/int.json

0

index.js

const { clone, status } = require('isomorphic-git')
const http = require('isomorphic-git/http/node')
const rimraf = require('rimraf')
const fs = require('fs')
const { strictEqual } = require('assert')

main().catch(console.error)

async function main () {
  const dir = './demo'
  const pfs = fs.promises

  rimraf.sync(dir)

  await pfs.mkdir(dir)

  await clone({
    fs, http, dir, url: 'https://github.com/alex996/git-int-test.git', ref: 'master',
  });

  let int = JSON.parse(await pfs.readFile(`${dir}/int.json`, 'utf-8')); // 0

  int++; // 1

  await pfs.writeFile(`${dir}/int.json`, JSON.stringify(int, null, 2) + '\n', 'utf-8')

  strictEqual(await status({ fs, dir, filepath: 'int.json' }), "*modified")
}

When I run node index.js, I get

+ actual - expected

+ 'unmodified'
- '*modified'
    at main (/home/alex/Contract/caravan/git-memdb-demo/index.js:27:3) {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: 'unmodified',
  expected: '*modified',
  operator: 'strictEqual'
}

Here are the deps in package.json

"dependencies": {
  "isomorphic-git": "^1.7.8",
  "rimraf": "^3.0.2"
}

Also here's my env:

$ node -v
v14.13.1

$ npm -v
6.14.8

$ lsb_release -a
No LSB modules are available.
Distributor ID:	Ubuntu
Description:	Ubuntu 20.04.1 LTS
Release:	20.04
Codename:	focal

Strangely enough, double-digit integers are registered correctly: int = 10;. I didn't notice this with strings, only integers so far.

Initially, I ran into this with objects that had integer props, something like: { "int": 0 }, hence the use of the JSON API.

Any idea where the culprit might be? Thanks

[alex996]

We found that if you manually run cd ./demo; git status; cd .. from the terminal, then change the main function to

async function main () {
  const dir = './demo'
  strictEqual(await status({ fs, dir, filepath: 'int.json' }), "*modified")
}

and re-run node index.js - it will pass the assertion as expected.

This is probably because git status updates ./demo/.git/index. So, as a workaround, we're using git.resetIndex:

const { clone, resetIndex, status } = require('isomorphic-git')

// ...

await pfs.writeFile(`${dir}/int.json`, JSON.stringify(int, null, 2) + '\n', 'utf-8')

await resetIndex({ fs, dir, filepath: 'int.json' }) // <----------

strictEqual(await status({ fs, dir, filepath: 'int.json' }), "*modified") // works

[alex996]

Something else I noticed is if I run vim ./demo/int.json and save it using wq (without any modification, just open and save) - then when I re-run node index.js with just

async function main () {
  const dir = './demo'
  strictEqual(await status({ fs, dir, filepath: 'int.json' }), "*modified")
}

it passes as well. It still fails if I simply access the file with cat or vim + q. So, it seems that a "dry" write does the trick (why?).

Perhaps I need to pass a particular mode or flag to fs.writeFile() so as to emulate a "native" write like in vim? In another test, I also compared buffer contents before and after using Node REPL, and they came out identical (using Buffer.equals).

[jcubic]

I can confirm the issue. It may be hard to find the root cause.

[dead-end]

If you take the initial example and add a sleep of one second between the clone call and the update, then the example works:

const { clone, status } = require("isomorphic-git");
const http = require("isomorphic-git/http/node");
const rimraf = require("rimraf");
const fs = require("fs");
const { strictEqual } = require("assert");

main().catch(console.error);

async function main() {
  const dir = "./demo";
  const pfs = fs.promises;

  rimraf.sync(dir);

  await pfs.mkdir(dir);

  await clone({
    fs,
    http,
    dir,
    url: "https://github.com/alex996/git-int-test.git",
    ref: "master",
  });

  let int = JSON.parse(await pfs.readFile(`${dir}/int.json`, "utf-8")); // 0

  int++; // 1

  await sleep(1000);
  function sleep(ms) {
    return new Promise((resolve) => {
      setTimeout(resolve, ms);
    });
  }

  await pfs.writeFile(
    `${dir}/int.json`,
    JSON.stringify(int, null, 2) + "\n",
    "utf-8"
  );

  strictEqual(await status({ fs, dir, filepath: "int.json" }), "*modified");
}

I think the root cause of the issue is the compareStats function, which compares the file in the index with the file in the working directory:

import { normalizeStats } from './normalizeStats.js'

export function compareStats(entry, stats) {
  // Comparison based on the description in Paragraph 4 of
  // https://www.kernel.org/pub/software/scm/git/docs/technical/racy-git.txt
  const e = normalizeStats(entry)
  const s = normalizeStats(stats)
  const staleness =
    e.mode !== s.mode ||
    e.mtimeSeconds !== s.mtimeSeconds ||
    e.ctimeSeconds !== s.ctimeSeconds ||
    e.uid !== s.uid ||
    e.gid !== s.gid ||
    e.ino !== s.ino ||
    e.size !== s.size
  return staleness
}

The comparison function uses seconds (mtimeSeconds) to check if the files may differ and this is not precise enough for the example code. There are only milliseconds between the clone call and the update of the file.

The other example above says that it works with a 2 digit integer. In this case the size changes, which is detected correctly by the comparison function.

[CMCDragonkai]

I think there needs to be a better way to know whether a file is modified. It can't rely on time solely. Perhaps some other data that is logically stepped.

[dead-end]

The normalizeStats function also returns:

• ctimeNanoseconds
• mtimeNanoseconds

This should be pecise enought to detect differeces.

[jcubic]

I think that canonical git don't check timestamp of the file only file content. So this will never be implemented. Unless you can find example of working with git where file timestamp affect the git repo.

But OP code is different because content of the file changes.

[dead-end]

This is a link to the git-update-index description:

https://github.com/git/git/blob/master/Documentation/git-update-index.txt

In line 322 you can find:

USING ``ASSUME UNCHANGED'' BIT

Many operations in Git depend on your filesystem to have an
efficient lstat(2) implementation, so that st_mtime
information for working tree files can be cheaply checked to see
if the file contents have changed from the version recorded in
the index file. Unfortunately, some filesystems have
inefficient lstat(2). If your filesystem is one of them, you
can set "assume unchanged" bit to paths you have not changed to
cause Git not to do this check. Note that setting this bit on a
path does not mean Git will check the contents of the file to
see if it has changed -- it makes Git to omit any checking and
assume it has not changed. When you make changes to working
tree files, you have to explicitly tell Git about it by dropping
"assume unchanged" bit, either before or after you modify them.

[dead-end]

I have two questions:

What do you mean with So this will never be implemented.? The current implementation relies on the timestamps. The compareStats function is part of isomorphic git and checks timestamps to detect if a file in the working directory changed.

What is OP code?

[jcubic]

I though, based on your comment, that you want to make status return modified when time of the file is changed.

An OP code is Original Poster Code, which means code from the author of this issue.

[isomorphic-git-bot]

🎉 This issue has been resolved in version 1.24.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[jcubic]

The fix was invalid, Need to revert.

[coderaiser]

Proper fix #1769

[CMCDragonkai]

What's the status of this?

[jcubic]

@CMCDragonkai Do you want to contribute with a fix if you need this?

[CMCDragonkai]

Is this just a timing issue? We worked around with just a 1 ms delay.

[jcubic]

This is just a workaround, it will require debugging to find the root cause. Maybe some part of the code use setTimeout 0 or maybe there is some async function somewhere that is invoked in next tick.

I would try to debug first and see what is actually happening.