Bug 38048 - new-run-webkit-tests: Move _path_to_apache_config_file down to webkit.py
Summary: new-run-webkit-tests: Move _path_to_apache_config_file down to webkit.py
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords: NRWT
Depends on:
Blocks:
 
Reported: 2010-04-23 09:32 PDT by Tor Arne Vestbø
Modified: 2017-07-18 08:30 PDT (History)
4 users (show)

See Also:


Attachments
Patch (7.98 KB, patch)
2010-04-26 04:29 PDT, Tor Arne Vestbø
no flags Details | Formatted Diff | Diff
Move _path_to_apache_config_file down to webkit.py (7.98 KB, patch)
2010-04-26 04:48 PDT, Tor Arne Vestbø
eric: review-
Details | Formatted Diff | Diff
Add Apache configuration for SuSE based systems (33.45 KB, patch)
2010-04-26 04:49 PDT, Tor Arne Vestbø
eric: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tor Arne Vestbø 2010-04-23 09:32:12 PDT
Move _path_to_apache_config_file down to webkit.py
Comment 1 Tor Arne Vestbø 2010-04-26 04:29:11 PDT
Created attachment 54274 [details]
Patch
Comment 2 Tor Arne Vestbø 2010-04-26 04:48:33 PDT
Created attachment 54278 [details]
Move _path_to_apache_config_file down to webkit.py
Comment 3 Tor Arne Vestbø 2010-04-26 04:49:24 PDT
Created attachment 54279 [details]
Add Apache configuration for SuSE based systems
Comment 4 Eric Seidel (no email) 2010-04-26 11:24:02 PDT
Comment on attachment 54278 [details]
Move _path_to_apache_config_file down to webkit.py

This will break Chromium, since it does not inherit from webkit.py

Seems this should be a white-list for debian instead of a blacklist:
+            distro = platform.linux_distribution(full_distribution_name=False)[0]
+            # FIXME: This is not going to scale. We should find a better way to choose
+            # which config file to use, while still supporting the major distributions
+            if distro in ["SuSE", "fedora", "redhat", "centos", "mandrake",
+                          "mandriva", "rocks", "slackware", "yellowdog", "turbolinux"]:

Also, seems like we need a platform abstraction instead of a port in this case.
Comment 5 Eric Seidel (no email) 2010-04-26 11:27:13 PDT
Comment on attachment 54279 [details]
Add Apache configuration for SuSE based systems

What's the subtle difference with this file?

This is a good example of why it should be a whitelist instead of a blacklist. :)

The proliferation of http.conf files is insane.  We're to the point where it may make sense to generate them instead of check in 7 different ones w/ unknown differences between them.
Comment 6 Ojan Vafai 2010-04-26 11:34:07 PDT
(In reply to comment #5)
> The proliferation of http.conf files is insane.  We're to the point where it
> may make sense to generate them instead of check in 7 different ones w/ unknown
> differences between them.

Seriously, I only see benefit to generating this file. The current situation is insane, especially as the subtle differences can be the difference between http tests being flaky or not.
Comment 7 Tor Arne Vestbø 2010-04-28 00:26:32 PDT
(In reply to comment #4)
> (From update of attachment 54278 [details])
> This will break Chromium, since it does not inherit from webkit.py

Ouch. What's the plan there?

> Seems this should be a white-list for debian instead of a blacklist:
> +            distro =
> platform.linux_distribution(full_distribution_name=False)[0]
> +            # FIXME: This is not going to scale. We should find a better way
> to choose
> +            # which config file to use, while still supporting the major
> distributions
> +            if distro in ["SuSE", "fedora", "redhat", "centos", "mandrake",
> +                          "mandriva", "rocks", "slackware", "yellowdog",
> "turbolinux"]:
> 
> Also, seems like we need a platform abstraction instead of a port in this case.

Fully agree. The idea was to move it down to webkit.py for now, and once we see a pattern there we can start abstracting stuff.
Comment 8 Tor Arne Vestbø 2010-04-28 00:27:13 PDT
(In reply to comment #6)
> (In reply to comment #5)
> > The proliferation of http.conf files is insane.  We're to the point where it
> > may make sense to generate them instead of check in 7 different ones w/ unknown
> > differences between them.
> 
> Seriously, I only see benefit to generating this file. The current situation is
> insane, especially as the subtle differences can be the difference between http
> tests being flaky or not.

Agree, I'll try to look at this.
Comment 9 Eric Seidel (no email) 2010-04-28 00:45:17 PDT
Adam and my plan is that eventually WebKit will replace base.Port.  Eventually Chromium will look more and more like the rest of the WebKit ports we figure.  We've not talked this all out with dpranke yet, he may have thoughts.
Comment 10 Eric Seidel (no email) 2010-04-28 00:45:53 PDT
One of the big problems in webkitpy right now is that we have no "platform" abstraction.  We have two separate "port" abstractions, but no "platform" abstraction to handle os-level things.
Comment 11 Dirk Pranke 2010-05-03 16:35:15 PDT
(In reply to comment #7)
> (In reply to comment #4)
> > (From update of attachment 54278 [details] [details])
> > This will break Chromium, since it does not inherit from webkit.py
> 
> Ouch. What's the plan there?

For now, if you want a single implementation of this function, at least put it in base.py instead of webkit.py.
Comment 12 Dirk Pranke 2010-05-03 16:35:47 PDT
(In reply to comment #9)
> Adam and my plan is that eventually WebKit will replace base.Port.  Eventually
> Chromium will look more and more like the rest of the WebKit ports we figure. 
> We've not talked this all out with dpranke yet, he may have thoughts.

This certainly seems reasonable.
Comment 13 Dirk Pranke 2010-05-03 16:36:09 PDT
(In reply to comment #9)
> Adam and my plan is that eventually WebKit will replace base.Port.  Eventually
> Chromium will look more and more like the rest of the WebKit ports we figure. 
> We've not talked this all out with dpranke yet, he may have thoughts.

This certainly seems reasonable.
Comment 14 Dirk Pranke 2010-05-03 16:40:43 PDT
(In reply to comment #10)
> One of the big problems in webkitpy right now is that we have no "platform"
> abstraction.  We have two separate "port" abstractions, but no "platform"
> abstraction to handle os-level things.

They intentionally aren't two orthogonal concepts, because they (theoretically) can't be. The "port" depends on the "platform", but the generic code intentionally only depended on "port" for anything that a port might need to implement - this was believed to be important for ports that may be fairly non-standard. The test.py platform is a simple example of this - it allows you to stub out or mock anything that might need to be overridden in a single place to be able to run the full test harness.

If you used an embedded/cross-compiling metaphor, you could imagine a "host" platform versus a "target" platform (which is sort of what rebaseline-webkit-tests  uses) and could abstract things out that way. I don't know if that would actually make things easier or harder to use and test.
Comment 15 Dirk Pranke 2012-07-17 17:57:40 PDT
is this bug still relevant? I'm actually now merging WebKitPort into Port (and Chromium inherits from WebKitPort.