Hi all,

I've run into the "same URLs with different parameters" bug again.  I've
reproduced it something like four times in a row.  In looking at the URL code,
it's not clear to me why this is considered a bug.  Why not just download the
first copy first, and then either copy it to the second filename or just
download it a second time?  We could track down why darcs is downloading this
file twice (you can see that I've added more debug information to the debug
message), but I think we're better off simply making the URL framework more
robust, since sooner or later someone is going to try downloading the same file
twice again, and the bug message will just pop up again.

David

$ time darcs push
Pushing to "darcy.physics.oregonstate.edu:darcs-central"...
darcs: bug in darcs!
URL.urlThread: same URLs with different parameters
http://darcs.net/_darcs/patches/0000000326-1a009975632a59cf2885b9b79d977ee5a74020d2924520fcfeff8495cc461a5a
./darcs797("./darcs797","/home/droundy/.darcs/cache/patches/0000000326-1a009975632a59cf2885b9b79d977ee5a74020d2924520fcfeff8495cc461a5a",Cachable,Cachable)
at src/URL.hs:152 compiled Oct  2 2008 17:52:58
This is a prerelease version of darcs.
You can check to see if this bug is already known at http://bugs.darcs.net/
If it is not, please report this to bugs@darcs.net
If possible include the output of 'darcs --exact-version'.

On Tue, Oct 07, 2008 at 14:42:46 -0000, David Roundy wrote:
> I've run into the "same URLs with different parameters" bug again.  I've
> reproduced it something like four times in a row.

This sounds like something I would like to see in the 2.1 release
(since it's an HTTP bugfix release among other things)

Are there any major risks involved in changing the behaviour the
way you describe (this close to end-game, that is)?

On Tue, Oct 07, 2008 at 02:56:19PM -0000, Eric Kow wrote:
> On Tue, Oct 07, 2008 at 14:42:46 -0000, David Roundy wrote:
> > I've run into the "same URLs with different parameters" bug again.  I've
> > reproduced it something like four times in a row.
> 
> This sounds like something I would like to see in the 2.1 release
> (since it's an HTTP bugfix release among other things)
> 
> Are there any major risks involved in changing the behaviour the
> way you describe (this close to end-game, that is)?

No risks as far as I can see, but I'd like Dmitry's opinion on the
subject, as I'm not very familiar with this code.  The advantage of
the local change (which just grabs the one file first) is that it
won't be triggered except when users would have run into the bug, so I
don't see any major risk.  On the other hand, if there is a more
correct solution, that would be better.

David

I'll add that I'm no longer able to reproduce this bug.  I tried
checking whether on (a separate copy of the repository) I could
reproduce it accessing http://darcs.net/unstable, and it disappeared,
and wouldn't come back.  Of course, there's tricky interaction with
the global cache, and I didn't think to back up my global cache before
the test.

But I think we know what needs fixing:  we should make it so that this
situation is no longer a bug.  I don't know how it arose, but that
shouldn't matter, as far as I can see.

David

I have just sent a patch for this. It should be safe for stable. Reviews are
welcomed of course.

Originally this code appeared because it seemed to me that if darcs tries to
download same url to different files it is an indication of a bug somewhere. But
if David says it is not, I believe him.

Note that there is still check for Cachable parameter. Perhaps this should be
removed as well, but it is not clear what chache parameter to choose for
download - the old one or the new one (or a more complex logic is needed).

Regards,
  Dmitry

BTW it would be great if someone can test the patch. I am not able to reproduce
the issue.

On Wed, Oct 08, 2008 at 12:01:27PM -0000, Dmitry Kurochkin wrote:
> I have just sent a patch for this. It should be safe for stable. Reviews are
> welcomed of course.

Yes, it looks good.

> Originally this code appeared because it seemed to me that if darcs tries to
> download same url to different files it is an indication of a bug somewhere. But
> if David says it is not, I believe him.

It may indicate an infelicity in the code, where we could be
optimizing better, but I wouldn't say it's a bug.  It can happen for
instance if someone called fetchFilePS on a file that had previously
been speculated.  This may be a mistake, but it needn't be.

> Note that there is still check for Cachable parameter. Perhaps this should be
> removed as well, but it is not clear what chache parameter to choose for
> download - the old one or the new one (or a more complex logic is needed).

I'd say we should just err on the side of less caching in this case.
I'd prefer to have defensive programming, so that darcs doesn't crash
as long as there is something reasonable to do.  The only reason for
using Cachable is to make things more efficient, so if a file is
requested twice with different values, there is a conservative
approach, and we can just take that approach.

David

Resolved by:

Wed Oct  8 15:30:40 MSD 2008  Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
  * Resolve issue1131: accept download requests for different files.
Wed Oct  8 17:43:08 MSD 2008  Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
  * Use copyFile+renameFile for safe copy in URL.waitNextUrl.