RESOLVED FIXED81028
nrwt: run with no args on lion, is putting baselines in platform/mac-lion, not platform/mac
https://bugs.webkit.org/show_bug.cgi?id=81028
Summary nrwt: run with no args on lion, is putting baselines in platform/mac-lion, no...
Dirk Pranke
Reported 2012-03-13 13:20:42 PDT
mac-lion shouldn't be automatically used for baselines until Mountain Lion ships and the apple guys decide that 'mac' == 'mountain lion'. platform/mac-lion should be included in the search path for baseline retrieval, but we should only be able to put new baselines there by hand (explicitly moving them from platform/mac by those who know if the baseline is different between Lion and Mountain Lion.
Attachments
Patch (8.26 KB, patch)
2012-03-13 17:17 PDT, Dirk Pranke
no flags
don't hardcode mac-lion (8.48 KB, patch)
2012-03-13 18:59 PDT, Dirk Pranke
ojan: review+
Dirk Pranke
Comment 1 2012-03-13 13:21:56 PDT
Looks like we broke this either in r92882 or r105951.
Alexey Proskuryakov
Comment 2 2012-03-13 13:26:43 PDT
We do not intend to put any results in mac-lion by hand until Mountain Lion ships.
Dirk Pranke
Comment 3 2012-03-13 15:42:49 PDT
(In reply to comment #2) > We do not intend to put any results in mac-lion by hand until Mountain Lion ships. Does that mean that you don't need there to be a mac-lion directory at all?
Dirk Pranke
Comment 4 2012-03-13 15:49:58 PDT
The last time I discussed this (w/ Darin, I believe), I thought there was a need for apple employees to be able to distinguish between 'mac-future' and 'mac-lion' for some period of time prior to Mountain Lion actually shipping (technically, it was Lion at the time). It may have been that the only thing in mac-lion should be a Skipped file? I think the most recent discussion of this was in https://bugs.webkit.org/show_bug.cgi?id=72748 . cf. also http://trac.webkit.org/wiki/LayoutTestsSearchPath ... If there will not be any baselines in mac-lion until ML ships, that simplifies the code slightly (mac-lion doesn't need to be in the baseline path for reads or writes, not just writes), which is why I am asking ...
Dirk Pranke
Comment 5 2012-03-13 17:17:47 PDT
Dirk Pranke
Comment 6 2012-03-13 17:23:14 PDT
ap, darin: note the tests in test_skipped_file_search_paths() and test_baseline_search_path() and make sure that they are what you'd like the behavior to be (hopefully the tests are close enough to english that their intent will be clear). If they aren't, let me know and I'll be happy to change them. In English, my expectations are: by default, on lion (platform mac-lion), we will look for baselines in mac-lion and then mac, but write new baselines into mac 'mac-future' will only read from 'mac' and will write to 'mac'. Nothing should ever be written to 'platform/mac-future'. There is no 'platform/mac-future/Skipped', and there are no baselines.
Alexey Proskuryakov
Comment 7 2012-03-13 17:34:33 PDT
> In English, my expectations are: > > by default, on lion (platform mac-lion), we will look for baselines in mac-lion and then mac, but write new baselines into mac This looks right to me. > 'mac-future' will only read from 'mac' and will write to 'mac'. > > Nothing should ever be written to 'platform/mac-future'. There is no 'platform/mac-future/Skipped', and there are no baselines. I don't know what mac-future platform is needed for, but this behavior sounds safe.
Mark Rowe (bdash)
Comment 8 2012-03-13 17:43:59 PDT
Comment on attachment 131757 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=131757&action=review > Tools/Scripts/webkitpy/layout_tests/port/mac.py:56 > + if self.name() == 'mac-lion': > + # mac-lion should normally write baselines into platform/mac until Mountain Lion ships. > + # It can still read baselines from platform/mac-lion, though. Can we avoid hard-coding Lion / Mountain Lion in here? This will always be the case for the currently-shipping version of the OS.
Dirk Pranke
Comment 9 2012-03-13 18:15:58 PDT
(In reply to comment #8) > (From update of attachment 131757 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=131757&action=review > > > Tools/Scripts/webkitpy/layout_tests/port/mac.py:56 > > + if self.name() == 'mac-lion': > > + # mac-lion should normally write baselines into platform/mac until Mountain Lion ships. > > + # It can still read baselines from platform/mac-lion, though. > > Can we avoid hard-coding Lion / Mountain Lion in here? This will always be the case for the currently-shipping version of the OS. I can certainly delete the "Mountain Lion" reference, and rework this so that we have a mac.CURRENT_VERSION='lion' or something - is that what you were suggesting? Or were you suggesting that somehow I attempt to figure out what "the currently shipping version of the OS" is dynamically? I don't know how I'd do that.
Dirk Pranke
Comment 10 2012-03-13 18:18:10 PDT
(In reply to comment #7) > > In English, my expectations are: > > > > by default, on lion (platform mac-lion), we will look for baselines in mac-lion and then mac, but write new baselines into mac > > This looks right to me. > > > 'mac-future' will only read from 'mac' and will write to 'mac'. > > > > Nothing should ever be written to 'platform/mac-future'. There is no 'platform/mac-future/Skipped', and there are no baselines. > > I don't know what mac-future platform is needed for, but this behavior sounds safe. 'mac-future' as a name/concept is needed for you guys to be able to run on unreleased versions of the operating system and have that get handled properly (distinguishing it from the currently shipping version). There isn't supposed to be a directory -- there is one currently checked in, but that's the result of a bug. Partially I am looking to confirm that the unreleased version of the OS doesn't need a Skipped file (at least not one that's checked into webkit).
Mark Rowe (bdash)
Comment 11 2012-03-13 18:19:12 PDT
(In reply to comment #10) > Partially I am looking to confirm that the unreleased version of the OS doesn't need a Skipped file (at least not one that's checked into webkit). That's correct.
Mark Rowe (bdash)
Comment 12 2012-03-13 18:20:44 PDT
(In reply to comment #9) > > Or were you suggesting that somehow I attempt to figure out what "the currently shipping version of the OS" is dynamically? I don't know how I'd do that. MacPort has a list of OS X versions in order of release. It seems relatively easy to derive the desired information from there.
Dirk Pranke
Comment 13 2012-03-13 18:22:50 PDT
(In reply to comment #12) > (In reply to comment #9) > > > > Or were you suggesting that somehow I attempt to figure out what "the currently shipping version of the OS" is dynamically? I don't know how I'd do that. > > MacPort has a list of OS X versions in order of release. It seems relatively easy to derive the desired information from there. Sure, that works as well.
Dirk Pranke
Comment 14 2012-03-13 18:59:52 PDT
Created attachment 131771 [details] don't hardcode mac-lion
Dirk Pranke
Comment 15 2012-03-14 11:53:45 PDT
Note You need to log in before you can comment on or make changes to this bug.