revert windows-specific locking changes that broke tests

This reverts windows-specific parts of 5a98f2d509
There were no code paths in common between windows and unix, so this
will return Windows to the old behavior.

The problem that the commit talks about has to do with multiple different
locations where git-annex can store annex object files, but that is not
too relevant to Windows anyway, because on windows the filesystem is always
treated as criplled and/or symlinks are not supported, so it will only
use one object location. It would need to be using a repo populated
in another OS to have the other object location in use probably.
Then a drop and get could possibly lead to a dangling lock file.

And, I was not able to actually reproduce that situation happening
before making that commit, even when I forced a race. So making these
changes on windows was just begging trouble..

I suspect that the change that caused the reversion is in
Annex/Content/Presence.hs. It checks if the content file exists,
and then called modifyContentDirWhenExists, which seems like it would
not fail, but if something deleted the content file at that point,
that call would fail. Which would result in an exception being thrown,
which should not normally happen from a call to inAnnexSafe. That was a
windows-specific change; the unix side did not have an equivilant
change.

Sponsored-by: Dartmouth College's Datalad project
This commit is contained in:
Joey Hess 2022-05-23 13:04:39 -04:00
parent 0aeb6e85f5
commit 478ed28f98
No known key found for this signature in database
GPG key ID: DB12DB0FF05F8F38
5 changed files with 34 additions and 2 deletions

View file

@ -177,7 +177,7 @@ posixLocker takelock lockfile = do
winLocker :: (LockFile -> IO (Maybe LockHandle)) -> ContentLocker winLocker :: (LockFile -> IO (Maybe LockHandle)) -> ContentLocker
winLocker takelock _ (Just lockfile) = winLocker takelock _ (Just lockfile) =
let lck = do let lck = do
modifyContentDirWhenExists lockfile $ modifyContentDir lockfile $
void $ liftIO $ tryIO $ void $ liftIO $ tryIO $
writeFile (fromRawFilePath lockfile) "" writeFile (fromRawFilePath lockfile) ""
liftIO $ takelock lockfile liftIO $ takelock lockfile

View file

@ -113,7 +113,7 @@ inAnnexSafe key = inAnnex' (fromMaybe True) (Just False) go key
- remove the lock file to clean up after ourselves. -} - remove the lock file to clean up after ourselves. -}
checklock (Just lockfile) contentfile = checklock (Just lockfile) contentfile =
ifM (liftIO $ doesFileExist (fromRawFilePath contentfile)) ifM (liftIO $ doesFileExist (fromRawFilePath contentfile))
( modifyContentDirWhenExists lockfile $ liftIO $ ( modifyContentDir lockfile $ liftIO $
lockShared lockfile >>= \case lockShared lockfile >>= \case
Nothing -> return is_locked Nothing -> return is_locked
Just lockhandle -> do Just lockhandle -> do

View file

@ -0,0 +1,14 @@
The changes to make `git-annex test` concurrent have
broken using eg `git-annex test -p 'concurrent get of dup key regression'`
The problem is that tasty is being run with a subset of tests in each
runner, so most of them don't know about the test they're being limited to
perform.
There either needs to be a way to disable concurrency (eg, run all tests
in one runner with -J1), or the code detect when tasty is limited, and
automatically disable concurrency.
Also, it looks like the repo setup test is not being run, even though it's
supposed to be a dependency of the test it was limited to.
--[[Joey]]

View file

@ -213,3 +213,5 @@ Windows 10 version 21H2 (build 19044.1706), 64 bit.
Git Annex is great. I use it several times a week with my multigigabyte backups, where it gives structure to my image-based backup routines, so you could say I'm a believer. :) Git Annex is great. I use it several times a week with my multigigabyte backups, where it gives structure to my image-based backup routines, so you could say I'm a believer. :)
[[!meta author=jkniiv]] [[!meta author=jkniiv]]
> [[fixed|done]] --[[Joey]]

View file

@ -0,0 +1,16 @@
[[!comment format=mdwn
username="joey"
subject="""comment 1"""
date="2022-05-23T16:57:34Z"
content="""
Thanks for another fine bug report.
I took a look at that commit, and it seems that reverting it for Windows
makes the most sense. There were two places where it changed behavior on
Windows, that were both separated from the code paths for other OS's.
I've made that change, which should fix it. I'm going to close this bug
report, but please follow up if you still see the issue.
(Also, I've opened [[tasty_test_limiting_broken_by_concurrency]]
"""]]