darcs

Patch 387 sort out darcs tag

Title sort out darcs tag
Superseder Nosy List dastapov, ganesh
Related Issues "darcs pull" regression with darcs-1 repos: all patches are being pulled each time
View: 1942
Status accepted Assigned To dastapov
Milestone

Created on 2010-09-11.18:53:26 by dastapov, last changed 2011-05-10.22:06:15 by darcswatch. Tracked on DarcsWatch.

Files
File name Status Uploaded Type Edit Remove
resolve-issue1942_-fix-an-io-interleaving-bug-in-old_fashioned-readrepo_.dpatch ganesh, 2010-10-11.20:56:38 text/x-darcs-patch
resolve-issue1942_-fix-the-fix-which-ended-up-too-strict-due-to-unsealing_.dpatch dastapov, 2010-09-11.18:53:26 text/x-darcs-patch
resolve-issue1942_-fix-the-fix-which-ended-up-too-strict-due-to-unsealing_.dpatch ganesh, 2010-10-04.18:43:15 text/x-darcs-patch
unnamed dastapov, 2010-09-11.18:53:26
unnamed ganesh, 2010-10-04.18:43:15
unnamed ganesh, 2010-10-11.20:56:38
See mailing list archives for discussion on individual patches.
Messages
msg12529 (view) Author: dastapov Date: 2010-09-11.18:53:26
1 patch for repository http://darcs.net/repos/unstable:

Sat Sep 11 21:53:55 EEST 2010  Dmitry Astapov <dastapov@gmail.com>
  * Resolve issue1942: fix the fix which ended up too strict due to unsealing.
  (manually backpopting fix by Petr Rockai to mainline)
Attachments
msg12532 (view) Author: tux_rocker Date: 2010-09-12.12:44:20
I tried it out and it seems to cause failures in the shell tests.
Sometimes, "darcs tag" fails with an error like:

darcs:
/tmp/tmp7075/temp1/_darcs/patches/20100912123535-ab4d8-3b4a494b8622b7b2014891b326293bb76a6e7ab2.gz:
openBinaryFile: does not exist (No such file or directory)
msg12630 (view) Author: ganesh Date: 2010-10-03.21:07:03
For me, with the patch applied to latest HEAD, 'cabal test issue1248' 
consistently fails, whereas 'dist/build/darcs-test/darcs-test --
tests=issue1248' consistently succeeds.
msg12631 (view) Author: mornfall Date: 2010-10-03.21:21:46
Ganesh Sittampalam <bugs@darcs.net> writes:
> For me, with the patch applied to latest HEAD, 'cabal test issue1248' 
> consistently fails, whereas 'dist/build/darcs-test/darcs-test --
> tests=issue1248' consistently succeeds.

This is likely because the latter will pick up the darcs on $PATH, which
is probably not the same as the one you have just built. You also need
to pass --darcs path/to/darcs to test a specific binary. (Maybe it is
worth printing a warning if --darcs is not given explicitly...)

Yours,
   Petr.
msg12632 (view) Author: ganesh Date: 2010-10-04.07:35:34
Petr Rockai wrote:
> Ganesh Sittampalam <bugs@darcs.net> writes:
>> For me, with the patch applied to latest HEAD, 'cabal test issue1248'
>> consistently fails, whereas 'dist/build/darcs-test/darcs-test --
>> tests=issue1248' consistently succeeds.
> 
> This is likely because the latter will pick up the darcs on $PATH,
> which is probably not the same as the one you have just built. You
> also need to pass --darcs path/to/darcs to test a specific binary.
> (Maybe it is worth printing a warning if --darcs is not given
> explicitly...)    

I think it should fail completely; the test harness should never pick up
the darcs in the environment by default.

Anyway, I've now got a fix for the darcs tag failure, though I don't
fully understand why it was ever working! It's trivial; adding a patch
tries to optimize the inventory when the patch is a tag, but the
sequence of actions is "add patch to inventory" ; "optimize"; "write
patch file out". Reordering the last two steps solves the problem,
though it does imply we are reading a patch file when we shouldn't be.

I'll send it in this evening.

Ganesh

=============================================================================== 
Please access the attached hyperlink for an important electronic communications disclaimer: 
http://www.credit-suisse.com/legal/en/disclaimer_email_ib.html 
===============================================================================
msg12635 (view) Author: ganesh Date: 2010-10-04.18:43:15
2 patches for repository http://darcs.net:

Sat Sep 11 19:53:55 BST 2010  Dmitry Astapov <dastapov@gmail.com>
  * Resolve issue1942: fix the fix which ended up too strict due to unsealing.
  (manually backpopting fix by Petr Rockai to mainline)

Mon Oct  4 06:47:13 BST 2010  Ganesh Sittampalam <ganesh@earth.li>
  * write out tag before trying to optimize inventory
  This fixes 'darcs tag' for old-fashioned repos.
  The original code seems obviously wrong, so it's hard to understand
  why things only started breaking after this patch:
  
    Sat Sep 11 19:53:55 BST 2010  Dmitry Astapov <dastapov@gmail.com>
     * Resolve issue1942: fix the fix which ended up too strict due to unsealing.
     (manually backpopting fix by Petr Rockai to mainline)
Attachments
msg12637 (view) Author: mornfall Date: 2010-10-04.19:03:16
Ganesh Sittampalam <bugs@darcs.net> writes:

Resolve issue1942: fix the fix which ended up too strict due to unsealing.
--------------------------------------------------------------------------
> Dmitry Astapov <dastapov@gmail.com>**20100911185355
>  Ignore-this: 87012509b3c1c0137306b348ae585059
>  (manually backpopting fix by Petr Rockai to mainline)
> ] hunk ./src/Darcs/Repository/DarcsRepo.lhs 108
>  import Darcs.Utils ( catchall )
>  import Darcs.ProgressPatches ( progressFL )
>  import Printer ( text, (<>), Doc, ($$), empty )
> -import Darcs.Witnesses.Sealed ( Sealed(Sealed), seal, unseal )
> +import Darcs.Witnesses.Sealed ( Sealed(Sealed), seal, unseal, mapSeal )
>  
>  #include "impossible.h"
>  \end{code}
> hunk ./src/Darcs/Repository/DarcsRepo.lhs 315
>            parse2 :: RepoPatch p => PatchInfo -> FilePath
>                                  -> IO (Sealed (PatchInfoAnd p C(x)))
>            parse2 i fn = do ps <- unsafeInterleaveIO $ gzFetchFilePS fn Cachable
> -                           Sealed p <- return $ hopefullyNoParseError (toPath fn) (readPatch ps)
> -                           return $ seal $ patchInfoAndPatch i p
> +                           return $ patchInfoAndPatch i
> +                             `mapSeal` hopefullyNoParseError (toPath fn) (readPatch ps)
>            hopefullyNoParseError :: String -> Maybe (Sealed (Named a1dr C(x)), b)
>                                  -> Sealed (Hopefully (Named a1dr) C(x))
>            hopefullyNoParseError _ (Just (Sealed x, _)) = seal $ actually x

Not a new patch, although I am not sure about it's review status.

write out tag before trying to optimize inventory
-------------------------------------------------

> Ganesh Sittampalam <ganesh@earth.li>**20101004054713
>  Ignore-this: aebf3d7ace08cce188501432653fd2b7
>  This fixes 'darcs tag' for old-fashioned repos.
>  The original code seems obviously wrong, so it's hard to understand
>  why things only started breaking after this patch:
>  
>    Sat Sep 11 19:53:55 BST 2010  Dmitry Astapov <dastapov@gmail.com>
>     * Resolve issue1942: fix the fix which ended up too strict due to unsealing.
>     (manually backpopting fix by Petr Rockai to mainline)
>  
> ] hunk ./src/Darcs/Repository/DarcsRepo.lhs 193
>  addToTentativeInventory compr p =
>      do appendDocBinFile (darcsdir++"/tentative_inventory") $ text "\n"
>                              <> showPatchInfo (patch2patchinfo p)
> +       res <- writePatch compr p
>         when (isTag $ patch2patchinfo p) $
>              do debugMessage "Optimizing the tentative inventory, since we're adding a tag."
>                 realdir <- toPath `fmap` ioAbsoluteOrRemote "."
> hunk ./src/Darcs/Repository/DarcsRepo.lhs 202
>                 Sealed ps <- readRepoPrivate k realdir "tentative_inventory"
>                              :: IO  (SealedPatchSet p C(Origin) )
>                 simplyWriteInventory "tentative_inventory" "." $ slightlyOptimizePatchset ps
> -       writePatch compr p
> +       return res
>  
>  addToTentativePristine :: Effect p => p C(x y) -> IO ()
>  addToTentativePristine p =

The patch looks OK to me. Please do push if it passes tests (assuming
the first patch is OK).

Yours,
   Petr.
msg12638 (view) Author: dastapov Date: 2010-10-04.19:22:03
FWIW, this patch works for me. What I mean is, darcs passes all tests 
(which is obviously expected), but it also does not bring back patch-
pulling behavior.

Thank you, Ganesh!
msg12640 (view) Author: ganesh Date: 2010-10-04.20:21:20
I've marked this as ready to go in on the basis of the discussion so far, 
which I think constitutes sufficient review.
msg12645 (view) Author: darcswatch Date: 2010-10-04.21:06:57
This patch bundle (with 1 patches) was just applied to the repository http://darcs.net/.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_.html#bundle-400a8fffcf4752136c03b57f01b72e9a56467cca
msg12646 (view) Author: darcswatch Date: 2010-10-04.21:07:01
This patch bundle (with 2 patches) was just applied to the repository http://darcs.net/.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_.html#bundle-5367d599bed489c3d56fe24f0912e3235d528754
msg12682 (view) Author: tux_rocker Date: 2010-10-11.07:14:11
The "write out tag before trying to optimize inventory" does not apply
cleanly to the 2.5 branch.
msg12684 (view) Author: ganesh Date: 2010-10-11.07:49:57
Reinier Lamers wrote:
> Reinier Lamers <tux_rocker@reinier.de> added the comment:
> 
> The "write out tag before trying to optimize inventory" does not
> apply cleanly to the 2.5 branch. 

Sorry, I should have made it against that to begin with.

I'll make a fresh version for the 2.5 branch. Are we trying to merge all
of 2.5 to HEAD?

Ganesh

=============================================================================== 
Please access the attached hyperlink for an important electronic communications disclaimer: 
http://www.credit-suisse.com/legal/en/disclaimer_email_ib.html 
===============================================================================
msg12686 (view) Author: ganesh Date: 2010-10-11.20:56:39
Here's a version of the patch for 2.5. Only the third patch has been
amended.

3 patches for repository http://darcs.net/releases/branch-2.5:

Sat Sep  4 04:09:47 BST 2010  Petr Rockai <me@mornfall.net>
  * Resolve issue1942: Fix an IO interleaving bug in old-fashioned readRepo.

Sat Sep 11 19:53:55 BST 2010  Dmitry Astapov <dastapov@gmail.com>
  * Resolve issue1942: fix the fix which ended up too strict due to unsealing.
  (manually backpopting fix by Petr Rockai to mainline)

Mon Oct 11 21:52:53 BST 2010  Ganesh Sittampalam <ganesh@earth.li>
  * write out tag before trying to optimize inventory
  
  This is a backport of the patch from HEAD to the 2.5 branch.
  
  This fixes 'darcs tag' for old-fashioned repos.
  The original code seems obviously wrong, so it's hard to understand
  why things only started breaking after this patch:
  
    Sat Sep 11 19:53:55 BST 2010  Dmitry Astapov <dastapov@gmail.com>
     * Resolve issue1942: fix the fix which ended up too strict due to unsealing.
     (manually backpopting fix by Petr Rockai to mainline)
Attachments
msg13143 (view) Author: darcswatch Date: 2010-11-21.13:13:22
This patch bundle (with 3 patches) was just applied to the repository http://darcs.net/.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_.html#bundle-40ae5070960d5cc1321bf5711e5e4fd2de8ef81a
msg14281 (view) Author: darcswatch Date: 2011-05-10.20:36:09
This patch bundle (with 3 patches) was just applied to the repository http://darcs.net/reviewed.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_reviewed.html#bundle-40ae5070960d5cc1321bf5711e5e4fd2de8ef81a
msg14305 (view) Author: darcswatch Date: 2011-05-10.21:05:55
This patch bundle (with 1 patches) was just applied to the repository http://darcs.net/reviewed.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_reviewed.html#bundle-400a8fffcf4752136c03b57f01b72e9a56467cca
msg14381 (view) Author: darcswatch Date: 2011-05-10.22:06:15
This patch bundle (with 2 patches) was just applied to the repository http://darcs.net/reviewed.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_reviewed.html#bundle-5367d599bed489c3d56fe24f0912e3235d528754
History
Date User Action Args
2010-09-11 18:53:26dastapovcreate
2010-09-11 18:53:40dastapovsetissues: + "darcs pull" regression with darcs-1 repos: all patches are being pulled each time
2010-09-11 18:55:03darcswatchsetdarcswatchurl: http://darcswatch.nomeata.de/repo_http:__darcs.net_.html#bundle-400a8fffcf4752136c03b57f01b72e9a56467cca
2010-09-12 12:44:20tux_rockersetstatus: needs-review -> followup-requested
assignedto: dastapov
messages: + msg12532
2010-10-03 21:07:03ganeshsetnosy: + ganesh
messages: + msg12630
2010-10-03 21:21:46mornfallsetmessages: + msg12631
2010-10-04 07:35:35ganeshsetmessages: + msg12632
title: Resolve issue1942: fix the fix which ended up too stri... -> Resolve issue1942: fix the fix whichended up too stri...
2010-10-04 18:43:16ganeshsetfiles: + resolve-issue1942_-fix-the-fix-which-ended-up-too-strict-due-to-unsealing_.dpatch, unnamed
messages: + msg12635
title: Resolve issue1942: fix the fix whichended up too stri... -> sort out darcs tag
2010-10-04 18:44:12darcswatchsetdarcswatchurl: http://darcswatch.nomeata.de/repo_http:__darcs.net_.html#bundle-400a8fffcf4752136c03b57f01b72e9a56467cca -> http://darcswatch.nomeata.de/repo_http:__darcs.net_.html#bundle-5367d599bed489c3d56fe24f0912e3235d528754
2010-10-04 19:03:16mornfallsetmessages: + msg12637
2010-10-04 19:22:03dastapovsetmessages: + msg12638
2010-10-04 20:20:36ganeshsetstatus: followup-requested -> accepted-pending-tests
2010-10-04 20:21:20ganeshsetmessages: + msg12640
2010-10-04 21:06:57darcswatchsetstatus: accepted-pending-tests -> accepted
messages: + msg12645
2010-10-04 21:07:01darcswatchsetmessages: + msg12646
2010-10-11 07:14:11tux_rockersetstatus: accepted -> followup-requested
messages: + msg12682
2010-10-11 07:49:58ganeshsetmessages: + msg12684
2010-10-11 20:56:39ganeshsetfiles: + resolve-issue1942_-fix-an-io-interleaving-bug-in-old_fashioned-readrepo_.dpatch, unnamed
messages: + msg12686
2010-10-11 20:57:18darcswatchsetdarcswatchurl: http://darcswatch.nomeata.de/repo_http:__darcs.net_.html#bundle-5367d599bed489c3d56fe24f0912e3235d528754 -> http://darcswatch.nomeata.de/repo_http:__darcs.net_.html#bundle-40ae5070960d5cc1321bf5711e5e4fd2de8ef81a
2010-11-21 13:13:22darcswatchsetstatus: followup-requested -> accepted
messages: + msg13143
2011-05-10 20:36:09darcswatchsetmessages: + msg14281
2011-05-10 21:05:55darcswatchsetmessages: + msg14305
2011-05-10 22:06:15darcswatchsetmessages: + msg14381