r45939 broke my workflow
Hi. r45939 broke my workflow. Here's the related bugzilla bug: https://bugs.webkit.org/show_bug.cgi?id=26999 . Old "Roll out a patch" workflow: cd JavaScriptCore svn-create-patch > patch.txt svn-unapply patch.txt Old "Roll in a patch" workflow: cd JavaScriptCore svn-apply patch.txt These old ways of doing things no longer work because svn-apply and svn-unapply don't match svn-create-patch's new behavior of changing to the WebKit root directory if you're currently working in a WebKit subdirectory. Here's an example command-line interaction:
~/webkit/WebKitTools/Scripts$ svn-create-patch > ro.txt ~/webkit/WebKitTools/Scripts$ svn-unapply !$ svn-unapply ro.txt can't find file to patch at input line 5 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: WebKitTools/Scripts/webkitdirs.pm |=================================================================== |--- WebKitTools/Scripts/webkitdirs.pm (revision 47638) |+++ WebKitTools/Scripts/webkitdirs.pm (working copy) -------------------------- File to patch: ^C'WebKitTools/Scripts' is not a directory at ./svn- unapply line 315, <> line 9.
I tried to ignore this for a while, but it's really causing problems for me and at least one other WebKit developer. Should we revert r45939? Is there an easy fix to svn-apply and svn-unapply that you can make? Thanks, Geoff
On Friday, August 21, 2009 at 1:10:31 PM, Geoffrey Garen wrote:
r45939 broke my workflow. Here's the related bugzilla bug: https://bugs.webkit.org/show_bug.cgi?id=26999.
Old "Roll out a patch" workflow:
cd JavaScriptCore svn-create-patch > patch.txt svn-unapply patch.txt
Old "Roll in a patch" workflow:
cd JavaScriptCore svn-apply patch.txt
These old ways of doing things no longer work because svn-apply and svn-unapply don't match svn-create-patch's new behavior of changing to the WebKit root directory if you're currently working in a WebKit subdirectory. [...] I tried to ignore this for a while, but it's really causing problems for me and at least one other WebKit developer.
Should we revert r45939?
Is there an easy fix to svn-apply and svn-unapply that you can make?
I believe there is an easy fix for this particular workflow, which is to make svn-apply and svn-unapply behave the same way as svn-create-patch. This will make it slightly harder to apply patches that aren't meant to be applied at the repository root, but we could add a command-line switch to not change directories when applying. The switch would only be necessary if you're applying such a patch. IMO, patches that aren't created at the repository root are more confusing to review anyway. Dave
Is there an easy fix to svn-apply and svn-unapply that you can make?
I believe there is an easy fix for this particular workflow, which is to make svn-apply and svn-unapply behave the same way as svn-create-patch.
I’m a little irritated that we’re changing our Subversion scripts, svn- create-patch, svn-apply, and svn-unapply into WebKit-specific scripts. Previously, they were scripts that were independent of the particular project that enhanced Subversion in a non-project-specific way. I used the WebKit version on many projects other than the WebKit project. I think the names should change if we’re changing them to do WebKit- specific things. -- Darin
On Aug 21, 2009, at 8: 32PM, Darin Adler wrote:
I’m a little irritated that we’re changing our Subversion scripts, svn-create-patch, svn-apply, and svn-unapply into WebKit-specific scripts. Previously, they were scripts that were independent of the particular project that enhanced Subversion in a non-project- specific way. I used the WebKit version on many projects other than the WebKit project.
I think the names should change if we’re changing them to do WebKit- specific things.
I don't think the change I did before was WebKit specific. And the changes (not committed) to svn-[un]apply only reflect the changes to svn-create-patch, but are again not WebKit specific. These changes ensure that patches are created and applied from the root of any SVN Repository (which in fact is what git does by default and makes the most sense by removing any possible ambiguity or forcing someone to know which directory they should apply a patch). There has been some loss of functionality [1], but that can be fixed. [1] Create a patch without svn-create-patch which contains paths not from the repository's root and then attempting to use svn-apply or svn- unapply to deal with the patch. These are the only changes I know of. If there are any others its possible they may be WebKit specific. - Joseph Pecoraro
On Fri, Aug 21, 2009 at 5:57 PM, Joseph Pecoraro <joepeck02@gmail.com>wrote:
On Aug 21, 2009, at 8: 32PM, Darin Adler wrote:
I’m a little irritated that we’re changing our Subversion scripts, svn-create-patch, svn-apply, and svn-unapply into WebKit-specific scripts. Previously, they were scripts that were independent of the particular project that enhanced Subversion in a non-project-specific way. I used the WebKit version on many projects other than the WebKit project.
I think the names should change if we’re changing them to do WebKit-specific things.
I don't think the change I did before was WebKit specific. And the changes (not committed) to svn-[un]apply only reflect the changes to svn-create-patch, but are again not WebKit specific. These changes ensure that patches are created and applied from the root of any SVN Repository (which in fact is what git does by default and makes the most sense by removing any possible ambiguity or forcing someone to know which directory they should apply a patch). There has been some loss of functionality [1], but that can be fixed.
[1] Create a patch without svn-create-patch which contains paths not from the repository's root and then attempting to use svn-apply or svn-unapply to deal with the patch.
These are the only changes I know of. If there are any others its possible they may be WebKit specific.
Even if they were WebKit specific, I guess i don't see what the problem would be. After all, the tools are checked into WebKitTools in the webkit repository. Making them more useful for WebKit development at the expense of other development seems like an OK trade-off to me. Note that you could always grab a version of the files from back when they behaved the way you wanted and stick them in ~/bin (or some other dir on your path).
On Aug 21, 2009, at 6:15 PM, Jeremy Orlow wrote:
On Fri, Aug 21, 2009 at 5:57 PM, Joseph Pecoraro <joepeck02@gmail.com> wrote: On Aug 21, 2009, at 8: 32PM, Darin Adler wrote: I’m a little irritated that we’re changing our Subversion scripts, svn-create-patch, svn-apply, and svn-unapply into WebKit-specific scripts. Previously, they were scripts that were independent of the particular project that enhanced Subversion in a non-project- specific way. I used the WebKit version on many projects other than the WebKit project.
I think the names should change if we’re changing them to do WebKit- specific things.
I don't think the change I did before was WebKit specific. And the changes (not committed) to svn-[un]apply only reflect the changes to svn-create-patch, but are again not WebKit specific. These changes ensure that patches are created and applied from the root of any SVN Repository (which in fact is what git does by default and makes the most sense by removing any possible ambiguity or forcing someone to know which directory they should apply a patch). There has been some loss of functionality [1], but that can be fixed.
[1] Create a patch without svn-create-patch which contains paths not from the repository's root and then attempting to use svn-apply or svn-unapply to deal with the patch.
These are the only changes I know of. If there are any others its possible they may be WebKit specific.
Even if they were WebKit specific, I guess i don't see what the problem would be. After all, the tools are checked into WebKitTools in the webkit repository. Making them more useful for WebKit development at the expense of other development seems like an OK trade-off to me.
Note that you could always grab a version of the files from back when they behaved the way you wanted and stick them in ~/bin (or some other dir on your path).
FWIW Darin wrote the original CVS-based versions of these tools. That's not to say he owns their descendants forever, but telling him he should go to extraordinary lengths to make use of them for other purposes seems a little presumptuous. (No opinion on the factual question of whether the changes are WebKit specific or whether they are improvements for WebKit development; I'm not totally clear on the changes or their motivation.) Regards, Maciej
On Fri, Aug 21, 2009 at 6:25 PM, Maciej Stachowiak <mjs@apple.com> wrote:
On Aug 21, 2009, at 6:15 PM, Jeremy Orlow wrote:
On Fri, Aug 21, 2009 at 5:57 PM, Joseph Pecoraro <joepeck02@gmail.com>wrote:
On Aug 21, 2009, at 8: 32PM, Darin Adler wrote:
I’m a little irritated that we’re changing our Subversion scripts, svn-create-patch, svn-apply, and svn-unapply into WebKit-specific scripts. Previously, they were scripts that were independent of the particular project that enhanced Subversion in a non-project-specific way. I used the WebKit version on many projects other than the WebKit project.
I think the names should change if we’re changing them to do WebKit-specific things.
I don't think the change I did before was WebKit specific. And the changes (not committed) to svn-[un]apply only reflect the changes to svn-create-patch, but are again not WebKit specific. These changes ensure that patches are created and applied from the root of any SVN Repository (which in fact is what git does by default and makes the most sense by removing any possible ambiguity or forcing someone to know which directory they should apply a patch). There has been some loss of functionality [1], but that can be fixed.
[1] Create a patch without svn-create-patch which contains paths not from the repository's root and then attempting to use svn-apply or svn-unapply to deal with the patch.
These are the only changes I know of. If there are any others its possible they may be WebKit specific.
Even if they were WebKit specific, I guess i don't see what the problem would be. After all, the tools are checked into WebKitTools in the webkit repository. Making them more useful for WebKit development at the expense of other development seems like an OK trade-off to me.
Note that you could always grab a version of the files from back when they behaved the way you wanted and stick them in ~/bin (or some other dir on your path).
FWIW Darin wrote the original CVS-based versions of these tools. That's not to say he owns their descendants forever, but telling him he should go to extraordinary lengths to make use of them for other purposes seems a little presumptuous.
I wasn't aware of that. I guess that kind of does change things. :-)
On Friday, August 21, 2009 at 5:32:14 PM, Darin Adler wrote:
I’m a little irritated that we’re changing our Subversion scripts, svn-create-patch, svn-apply, and svn-unapply into WebKit-specific scripts. Previously, they were scripts that were independent of the particular project that enhanced Subversion in a non-project-specific way. I used the WebKit version on many projects other than the WebKit project.
The only dependency these scripts have is VSCUtils.pm, which contains common routines used by (almost) all the scripts. If these scripts are used elsewhere, VCSUtils.pm is the only additional file that needs to be copied with them. (VCSUtils.pm itself doesn't depend on anything else in WebKit as of r46669.) The only "non-svn" functionality they have now is that svn-apply is able to apply patches to either an svn or a git repository. (The other two scripts are still svn-only.) I'm not sure how this makes them WebKit-specific? Dave
I just committed http://trac.webkit.org/changeset/47675 which affects LayoutTest/accessibility/aria-roles.html The result is different on SnowLeopard than it is on Tiger/Leopard To account for this difference, I have an -expected file in platform/ mac-snowleopard where the line is < This test PASSES in DumpRenderTree. The role is AXRole: AXList There is an existing file in platform/mac/ where the line is
This test PASSES in DumpRenderTree. The role is AXRole: AXGroup
--- Now after committing, Tiger/Leopard are complaining and saying http://build.webkit.org/results/Tiger%20Intel%20Release/r47675%20(3758)/acce... that it is expecting my the expected file from mac-snowleopard instead of platform/mac My question is, why does Tiger/Leopard expect the file in the mac- snowleopard folder http://build.webkit.org/results/Tiger%20Intel%20Release/r47675%20(3758)/acce...
On Aug 22, 2009, at 10:33 AM, Chris Fleizach wrote:
I just committed
http://trac.webkit.org/changeset/47675
which affects LayoutTest/accessibility/aria-roles.html
The result is different on SnowLeopard than it is on Tiger/Leopard
To account for this difference, I have an -expected file in platform/ mac-snowleopard
where the line is
< This test PASSES in DumpRenderTree. The role is AXRole: AXList
There is an existing file in platform/mac/
where the line is
This test PASSES in DumpRenderTree. The role is AXRole: AXGroup
--- Now after committing, Tiger/Leopard are complaining and saying http://build.webkit.org/results/Tiger%20Intel%20Release/r47675%20(3758)/acce...
that it is expecting my the expected file from mac-snowleopard instead of platform/mac
My question is, why does Tiger/Leopard expect the file in the mac- snowleopard folder
http://build.webkit.org/results/Tiger%20Intel%20Release/r47675%20(3758)/acce...
Hi Chris, In layout test results, we make the latest Mac OS X version the rule, and earlier versions the exception. Tiger will look for results in mac- tiger first, then in mac-leopard, then in mac-snowleopard, then in mac, then finally in cross-platform results. Leopard will begin the search in mac-leopard, continue to mac-snowleopard, then mac, the cross-platform. As you can see, there are no expected results in mac-snowleopard (other than the ones you just added), because it’s the latest Mac OS X version. We will only start putting expected results in mac- snowleopard when the “latest” version (for which we put results in mac) will be something different. You should put the expected results for Snow Leopard in platform/mac (or, if they are cross-platform, alongside the test), and you should put the results for Leopard and earlier in platform/mac-leopard. —Dan
Hi Chris, In layout test results, we make the latest Mac OS X version the rule, and earlier versions the exception. Tiger will look for results in mac-tiger first, then in mac-leopard, then in mac-snowleopard, then in mac, then finally in cross-platform results. Leopard will begin the search in mac-leopard, continue to mac-snowleopard, then mac, the cross-platform. As you can see, there are no expected results in mac-snowleopard (other than the ones you just added), because it’s the latest Mac OS X version. We will only start putting expected results in mac-snowleopard when the “latest” version (for which we put results in mac) will be something different. You should put the expected results for Snow Leopard in platform/mac (or, if they are cross-platform, alongside the test), and you should put the results for Leopard and earlier in platform/mac-leopard. —Dan
Does this imply that if you've moved results from 'platform/mac' to 'platform/mac-leopard' when you switched from 10.5 to 10.6? (Since, presumable, some results that were in platform/mac were actually specific to 10.5?) I would've expected a different model, where if the output differed by version, then you had results in 'mac-leopard' and 'mac-snowleopard', and the presence in 'mac' meant it should be the same across both (this was the approach I was planning to use for Chromium, which is just starting to run into this problem with XP/Vista/Win 7). I can see some advantages to your approach, but it seems more confusing in the long run. -- Dirk
On Aug 23, 2009, at 11:28 AM, Dirk Pranke wrote:
Hi Chris, In layout test results, we make the latest Mac OS X version the rule, and earlier versions the exception. Tiger will look for results in mac- tiger first, then in mac-leopard, then in mac-snowleopard, then in mac, then finally in cross-platform results. Leopard will begin the search in mac-leopard, continue to mac-snowleopard, then mac, the cross- platform. As you can see, there are no expected results in mac-snowleopard (other than the ones you just added), because it’s the latest Mac OS X version. We will only start putting expected results in mac-snowleopard when the “latest” version (for which we put results in mac) will be something different. You should put the expected results for Snow Leopard in platform/ mac (or, if they are cross-platform, alongside the test), and you should put the results for Leopard and earlier in platform/mac-leopard. —Dan
Does this imply that if you've moved results from 'platform/mac' to 'platform/mac-leopard' when you switched from 10.5 to 10.6? (Since, presumable, some results that were in platform/mac were actually specific to 10.5?)
Yes, when the expected results of an existing test change under a new version of Mac OS X, legacy expected results are moved from platform/ mac to platform/mac-<last version with legacy behavior> and current expected results are put in platform/mac. <http://trac.webkit.org/changeset/47052
is an example.
I would've expected a different model, where if the output differed by version, then you had results in 'mac-leopard' and 'mac-snowleopard', and the presence in 'mac' meant it should be the same across both (this was the approach I was planning to use for Chromium, which is just starting to run into this problem with XP/Vista/Win 7).
Having platform/mac represent “latest and future versions of Mac OS X” means that when the next version appears, there is no need to do anything with tests whose results in the new version are the same as in the previous version. This model is analogous to how version- checking macros are used in WebKit code, by making exceptions for the past, not for the present: there is no BUILDING_ON_<latest version> macro.
I can see some advantages to your approach, but it seems more confusing in the long run.
I hope my explanation clears up the confusion. —Dan
On Sun, Aug 23, 2009 at 12:23 PM, Dan Bernstein<mitz@apple.com> wrote:
On Aug 23, 2009, at 11:28 AM, Dirk Pranke wrote:
Hi Chris, In layout test results, we make the latest Mac OS X version the rule, and earlier versions the exception. Tiger will look for results in mac-tiger first, then in mac-leopard, then in mac-snowleopard, then in mac, then finally in cross-platform results. Leopard will begin the search in mac-leopard, continue to mac-snowleopard, then mac, the cross-platform. As you can see, there are no expected results in mac-snowleopard (other than the ones you just added), because it’s the latest Mac OS X version. We will only start putting expected results in mac-snowleopard when the “latest” version (for which we put results in mac) will be something different. You should put the expected results for Snow Leopard in platform/mac (or, if they are cross-platform, alongside the test), and you should put the results for Leopard and earlier in platform/mac-leopard. —Dan
Does this imply that if you've moved results from 'platform/mac' to 'platform/mac-leopard' when you switched from 10.5 to 10.6? (Since, presumable, some results that were in platform/mac were actually specific to 10.5?)
Yes, when the expected results of an existing test change under a new version of Mac OS X, legacy expected results are moved from platform/mac to platform/mac-<last version with legacy behavior> and current expected results are put in platform/mac. <http://trac.webkit.org/changeset/47052> is an example.
Hi Dan, Thanks for the explanation, that definitely helps. Out of curiosity, when do you make the platform switch? For example, when did mac stop equalling leopard and start equaling snowleopard? It looks like we will adopt the same convention as you, for consistency's sake, and so I'd like to mirror it as closely as possible. -- Dirk
On Tue, Aug 25, 2009 at 4:38 PM, Dirk Pranke <dpranke@chromium.org> wrote:
Thanks for the explanation, that definitely helps. Out of curiosity, when do you make the platform switch? For example, when did mac stop equalling leopard and start equaling snowleopard?
"mac" doesn't "equal snowleopard" any more than it "equals leopard". Note that there's a mac-snowleopard folder right now, and test results in there would be used by any current Mac preferentially to those in "mac". "mac" just means "the results for when there isn't a different result set for 10.x and below". PK
On Aug 25, 2009, at 4:38 PM, Dirk Pranke wrote:
On Sun, Aug 23, 2009 at 12:23 PM, Dan Bernstein<mitz@apple.com> wrote:
On Aug 23, 2009, at 11:28 AM, Dirk Pranke wrote:
Hi Chris, In layout test results, we make the latest Mac OS X version the rule, and earlier versions the exception. Tiger will look for results in mac-tiger first, then in mac-leopard, then in mac-snowleopard, then in mac, then finally in cross-platform results. Leopard will begin the search in mac-leopard, continue to mac-snowleopard, then mac, the cross- platform. As you can see, there are no expected results in mac-snowleopard (other than the ones you just added), because it’s the latest Mac OS X version. We will only start putting expected results in mac-snowleopard when the “latest” version (for which we put results in mac) will be something different. You should put the expected results for Snow Leopard in platform/ mac (or, if they are cross-platform, alongside the test), and you should put the results for Leopard and earlier in platform/mac-leopard. —Dan
Does this imply that if you've moved results from 'platform/mac' to 'platform/mac-leopard' when you switched from 10.5 to 10.6? (Since, presumable, some results that were in platform/mac were actually specific to 10.5?)
Yes, when the expected results of an existing test change under a new version of Mac OS X, legacy expected results are moved from platform/mac to platform/mac-<last version with legacy behavior> and current expected results are put in platform/mac. <http://trac.webkit.org/changeset/47052
is an example.
Hi Dan,
Thanks for the explanation, that definitely helps. Out of curiosity, when do you make the platform switch? For example, when did mac stop equalling leopard and start equaling snowleopard?
The platform/mac-snowleopard directory was created in r41710 on 2009-03-14, and the required changes to the expected result search order were made in r41956 on 2009-03-24.
It looks like we will adopt the same convention as you, for consistency's sake, and so I'd like to mirror it as closely as possible.
-- Dirk
participants (10)
-
Chris Fleizach
-
Dan Bernstein
-
Darin Adler
-
David Kilzer
-
Dirk Pranke
-
Geoffrey Garen
-
Jeremy Orlow
-
Joseph Pecoraro
-
Maciej Stachowiak
-
Peter Kasting