Re: [webkit-dev] svn-create-patch acting oddly lately
On Mon, Jul 20, 2009 at 2:17 PM, Peter Kasting wrote: > I can switch svn-create-patch to using UUID if there are benefits over using > the Repository Root; it'd literally be a four-character change. I'm not enough of an SVN expert to know which is better. I'd prefer to use one method consistently throughout WebKitTools (ideally by calling into a common library). Adam (Apologies for not being on IRC, I'm in meetings at Berkeley today.) ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] svn-create-patch acting oddly lately
On Mon, Jul 20, 2009 at 2:52 PM, Darin Adler wrote: > On Jul 20, 2009, at 2:50 PM, Peter Kasting wrote: > > my ($newdir) = @_; >> my $before = Cwd::getcwd(); >> chdir $newdir; >> my $after = Cwd::getcwd(); >> >> That seems like it ought to work a lot better. >> > > Yes, it does. Hooray! Awesome. I'll get this patch up for review. Thanks for your patience Darin! PK ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] svn-create-patch acting oddly lately
On Jul 20, 2009, at 2:50 PM, Peter Kasting wrote: my ($newdir) = @_; my $before = Cwd::getcwd(); chdir $newdir; my $after = Cwd::getcwd(); That seems like it ought to work a lot better. Yes, it does. Hooray! -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] svn-create-patch acting oddly lately
On Mon, Jul 20, 2009 at 2:40 PM, Peter Kasting wrote: > It looks from here like the chdir call is causing curdir() to report the > non-symbolic-link form of things. If that's true, it seems like this > workaround would work: > > ... > my ($newdir) = @_; > my $before = File::Spec->rel2abs( File::Spec->curdir() ); > + chdir $before; # Try and force curdir() to report the directory without > symbolic links. > + $before = File::Spec->rel2abs( File::Spec->curdir() ); > chdir $newdir; > my $after = File::Spec->rel2abs( File::Spec->curdir() ); > ... > > Could you try this and see if it works? > > Also, any Perl hackers are welcome to chip in here on how the current code > should be improved. I just looked up docs on the File::Spec module and saw > them mentioning something about realpath() that might be relevant here... > Looking at this more, this code just looks wrong to me (although, like I said, I know roughly zero Perl, so my instincts are not trustworthy). Perhaps instead of the hack above, try adding "use Cwd;" to the top of the file and then changing this to: ... my ($newdir) = @_; my $before = Cwd::getcwd(); chdir $newdir; my $after = Cwd::getcwd(); ... That seems like it ought to work a lot better. If that solves your problem, I'll get this reviewed and checked in. PK ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] svn-create-patch acting oddly lately
On Jul 20, 2009, at 2:40 PM, Peter Kasting wrote: It looks from here like the chdir call is causing curdir() to report the non-symbolic-link form of things. If that's true, it seems like this workaround would work: Makes sense. ... my ($newdir) = @_; my $before = File::Spec->rel2abs( File::Spec->curdir() ); + chdir $before; # Try and force curdir() to report the directory without symbolic links. + $before = File::Spec->rel2abs( File::Spec->curdir() ); chdir $newdir; my $after = File::Spec->rel2abs( File::Spec->curdir() ); ... Could you try this and see if it works? Sadly, I tried it, and it didn’t have any effect. -- Darin PS: It’s OK with me if someone wants to rewrite in python if this really is a result of perl badness. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] svn-create-patch acting oddly lately
On Mon, Jul 20, 2009 at 2:24 PM, Darin Adler wrote: > The key here is that "/Users/darin" is a symbolic link to > "/Volumes/Home/darin" — you can probably reproduce this by putting the > checkout directory inside a symbolic link. > Doesn't help on my Windows box at least... all my checkouts are rooted at symbolic links. In determineSvnRoot(), $last is ".." and then $svnRoot ends up as > "/Users/darin/Safari/OpenSource/WebKit/..", which seems bad. Is rel2abs > supposed to handle that? Not sure. JoePeck and I have talked some on IRC since this code was from his patch. He says he's encountered rel2abs() behaving differently on different systems. I know so little about Perl that I'm not sure what to do with that info. In chdirAndGetDifference(), before is > "/Users/darin/Safari/OpenSource/WebKit" and after is > "/Volumes/Home/darin/Safari/OpenSource". That's definitely bad, and seems like the source of this issue. It looks from here like the chdir call is causing curdir() to report the non-symbolic-link form of things. If that's true, it seems like this workaround would work: ... my ($newdir) = @_; my $before = File::Spec->rel2abs( File::Spec->curdir() ); + chdir $before; # Try and force curdir() to report the directory without symbolic links. + $before = File::Spec->rel2abs( File::Spec->curdir() ); chdir $newdir; my $after = File::Spec->rel2abs( File::Spec->curdir() ); ... Could you try this and see if it works? Also, any Perl hackers are welcome to chip in here on how the current code should be improved. I just looked up docs on the File::Spec module and saw them mentioning something about realpath() that might be relevant here... PK ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] svn-create-patch acting oddly lately
On Jul 20, 2009, at 2:17 PM, Peter Kasting wrote: In the meantime, I've tried and failed to recreate Darin's problem locally, so I'm at the mercy of other people to report that they're having this problem. The key here is that "/Users/darin" is a symbolic link to "/Volumes/ Home/darin" — you can probably reproduce this by putting the checkout directory inside a symbolic link. In determineSvnRoot(), $last is ".." and then $svnRoot ends up as "/ Users/darin/Safari/OpenSource/WebKit/..", which seems bad. Is rel2abs supposed to handle that? In chdirAndGetDifference(), before is "/Users/darin/Safari/OpenSource/ WebKit" and after is "/Volumes/Home/darin/Safari/OpenSource". -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] svn-create-patch acting oddly lately
On Jul 20, 2009, at 1:38 PM, Peter Kasting wrote: On Mon, Jul 20, 2009 at 1:28 PM, Darin Adler wrote: svn: Path '../..' ends in '..', which is unsupported for this operation This warning was caused by a change of mine that I checked in a fix for at noon on Friday. It shouldn't still happen on an up-to-date svn-create-patch. If your svn-create-patch is at least r46054 and this is happening, something is wrong. I updated and indeed the new version no longer gives this warning. Index: ../../../../../Users/darin/Safari/OpenSource/WebKit/mac/ WebView/WebHTMLView.mm === --- ../../../../../Users/darin/Safari/OpenSource/WebKit/mac/WebView/ WebHTMLView.mm (revision 46038) +++ ../../../../../Users/darin/Safari/OpenSource/WebKit/mac/WebView/ WebHTMLView.mm (working copy) This seems bad. I haven't seen this locally but I could have caused it. Is there any svn repository for any of the directories above your WebKit checkout? No. If your checkout wasn't quite up-to-date above, does updating fix this? No. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] svn-create-patch acting oddly lately
On Mon, Jul 20, 2009 at 2:11 PM, Joseph Pecoraro wrote: > > Yes, I saw scm.py's solution literally yesterday while I was learning how > to use bugzilla-tool because I switched to git. In any case the newer > revision uses a much better approach then the one in this changelog > (Repository Root) and ultimately the best approach would be using the UUID > like you suggested. > I can switch svn-create-patch to using UUID if there are benefits over using the Repository Root; it'd literally be a four-character change. In the meantime, I've tried and failed to recreate Darin's problem locally, so I'm at the mercy of other people to report that they're having this problem. In case it helps, the relevant bugs where svn-create-patch was changed seem to be bug 26999 (source of the original change eseidel mentions) and bug 27323 (source of my updates to the recently-added root calculation code). PK ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] svn-create-patch acting oddly lately
It's kind of sad that we have so many ways of finding the SVN root. The code in that link isn't right. You need to look at the UUID to get the right answer. See scm.py. Yes, I saw scm.py's solution literally yesterday while I was learning how to use bugzilla-tool because I switched to git. In any case the newer revision uses a much better approach then the one in this changelog (Repository Root) and ultimately the best approach would be using the UUID like you suggested. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] svn-create-patch acting oddly lately
On Mon, Jul 20, 2009 at 2:03 PM, Adam Barth wrote: > It's kind of sad that we have so many ways of finding the SVN root. > The code in that link isn't right. You need to look at the UUID to > get the right answer. See scm.py. My update made us look at the Repository Root field instead of looking at the error code returned by svn info. It seems like in practice this should be as good as the Repository UUID field. Or do you think I should change it? Also, if you and/or Darin were on IRC, I could ask this without annoying all the other folks on webkit-dev@ :) PK ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] svn-create-patch acting oddly lately
It's kind of sad that we have so many ways of finding the SVN root. The code in that link isn't right. You need to look at the UUID to get the right answer. See scm.py. Adam On Mon, Jul 20, 2009 at 1:59 PM, Peter Kasting wrote: > On Mon, Jul 20, 2009 at 1:55 PM, Eric Seidel wrote: >> >> It could also be from http://trac.webkit.org/changeset/45939. > > Yes. Knowing Darin's repository version would help narrow this down. > I am trying different things to see if I can reproduce locally. Once I can > reproduce I will either fix the problem or back out the offending change > immediately. > If anyone else has this problem, and is up-to-date, please let me know here > or on IRC. I would love to find out that this is in fact just a problem of > an out-of-date checkout but somehow I doubt it. > PK > ___ > webkit-dev mailing list > webkit-dev@lists.webkit.org > http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev > > ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] svn-create-patch acting oddly lately
On Mon, Jul 20, 2009 at 1:55 PM, Eric Seidel wrote: > It could also be from http://trac.webkit.org/changeset/45939. Yes. Knowing Darin's repository version would help narrow this down. I am trying different things to see if I can reproduce locally. Once I can reproduce I will either fix the problem or back out the offending change immediately. If anyone else has this problem, and is up-to-date, please let me know here or on IRC. I would love to find out that this is in fact just a problem of an out-of-date checkout but somehow I doubt it. PK ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] svn-create-patch acting oddly lately
On Mon, Jul 20, 2009 at 1:28 PM, Darin Adler wrote: > My patches look like this now: > > $ svn-create-patch > svn: Path '../..' ends in '..', which is unsupported for this operation This warning was caused by a change of mine that I checked in a fix for at noon on Friday. It shouldn't still happen on an up-to-date svn-create-patch. If your svn-create-patch is at least r46054 and this is happening, something is wrong. Index: > ../../../../../Users/darin/Safari/OpenSource/WebKit/mac/WebView/WebHTMLView.mm > === > --- > ../../../../../Users/darin/Safari/OpenSource/WebKit/mac/WebView/WebHTMLView.mm > (revision 46038) > +++ > ../../../../../Users/darin/Safari/OpenSource/WebKit/mac/WebView/WebHTMLView.mm > (working copy) This seems bad. I haven't seen this locally but I could have caused it. Is there any svn repository for any of the directories above your WebKit checkout? If your checkout wasn't quite up-to-date above, does updating fix this? PK ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] svn-create-patch acting oddly lately
My patches look like this now: $ svn-create-patch svn: Path '../..' ends in '..', which is unsupported for this operation Index: ../../../../../Users/darin/Safari/OpenSource/WebKit/mac/WebView/ WebHTMLView.mm === --- ../../../../../Users/darin/Safari/OpenSource/WebKit/mac/WebView/ WebHTMLView.mm (revision 46038) +++ ../../../../../Users/darin/Safari/OpenSource/WebKit/mac/WebView/ WebHTMLView.mm (working copy) [...] $ Yuck! Do I need to file a bug about this? -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
