dvcs-autosync incorrectly records changes on a file `4913`

[rmayr]

dvcs-autosync incorrectly records changes on a file `4913`

[Dieterbe]

Hmm it seems that vim is to blame.
vim does this to provide the user desired behavior, so it canot be expected for vim users to disable this feature (if disabling it is even possible). Vim actually tries to create the file, starting with filename 4913, and adding 123 until it finds an available filename.

https://wiki.hpcc.msu.edu/display/Issues/4913+files+created+by+vim+and+home+directory+file+locking+issue
http://groups.google.com/group/vim_dev/browse_thread/thread/b29a5da44971638e/6656f4c73f38ceea?pli=1
https://bugzilla.redhat.com/show_bug.cgi?id=427711

I wonder what the right approach here should be:

• tell users to deal with it themselves, like tell them to put 4913 and maybe the first few additions of 123 in their .gitignore (not very clean?) and don't name their files like that.
• handle the case of a removed file a bit better? (i.e. dvcs-autosync would not do the git add if the file vanished again). maybe even do that silently (no warnings)

[rmayr]

(Hopefully) fixed with 82b4cc8.

[Dieterbe]

I can confirm your patch goes in the branch and returns from the process_IN_CREATE() function, when event.pathname no longer exists.

however, this does not cause aborting the "git add" command. dvcs-autosync starts the "git add" which fails.
You can verify this by setting the loglevel to logging.DEBUG, tailing the logfile, and then typing something like touch foobarbaz4 && rm foobarbaz4 in the monitored directory.

Furthermore, there are also race conditions.
If you make the function like this:

    def process_IN_CREATE(self, event):
        # sanity check - don't add file if it (no longer) exists in the file system!
        logging.debug("IN_CREATE : got create for path %s", event.pathname)
        logging.debug("IN_CREATE: CWD %s", os.getcwd())
        if not os.path.exists(event.pathname):
            logging.debug('IN_CREATE: Ignoring file create event on %s, as it (no longer) exists - it was probably created as a temporary file and immediately removed by the application', event.pathname)
            return
        import time
        time.sleep(1)
        if not os.path.exists(event.pathname):
            logging.debug('IN_CREATE: NEEDEDSLEEP Ignoring file create event on %s, as it (no longer) exists - it was probably created as a temporary file and immediately removed by the application', event.pathname)
            return

        self._queue_action(event, cmd_add, [event.pathname])

then you can some 'NEEDEDSLEEP' entries in the log. This means that with the unmodified code, it notices the file exists, queues the cmd_add action, but the file gets removed shortly after.

(To clarify: it happens that the cmd_add is scheduled and a "git add" gets executed after either of the os.path.exists branches executed successfully, i.e. no matter if it was the first (normal) one or the second one that demos the race condition)

[Dieterbe]

Thinking a bit about this further, I would say the actual "check if file still exists" check should not happen at this point, but rather at the point where the actual cmd_add is about to get executed. Because there is a big gap in time between these two which allows race conditions.
But even this suggested approach is vulnerable to race conditions, although arguably these will be less likely, the files will probably have settled down after a few seconds, but still no guarantees.
Maybe we should make the default cmd_add git add --ignore-missing

[Dieterbe]

hmm --ignore-missing only works in dry run mode. maybe --ignore-errors ?

[ngoonee]

Still an issue, and ignore-errors doesn't help because at the time of the add the file just does not exist. The error is pathspec '4913' did not match any files, which I think cannot be ignored by git

EDIT: actually something surprising is that the name '4913' in .gitignore isn't respected. I already have .swp there and .swp files do not cause a problem... some problem in the ignore?

EDIT2: Putting the gitignore pattern of *4913 works, but this is suboptimal (some files MAY end in 4913, after all). The pattern matching logic should probably be improved slightly, but for now I can use this workaround.