Don't allow overlapping source directories.

Don't allow overlapping source directories.

BUG=https://code.google.com/p/dart/issues/detail?id=17699
R=nweiz@google.com

Committed: https://code.google.com/p/dart/source/detail?r=34750

[nweiz, 2014-04-03 22:20:55]

lgtm

https://codereview.chromium.org/216593006/diff/1/sdk/lib/_internal/pub/lib/sr...
File sdk/lib/_internal/pub/lib/src/barback/web_socket_api.dart (right):

https://codereview.chromium.org/216593006/diff/1/sdk/lib/_internal/pub/lib/sr...
sdk/lib/_internal/pub/lib/src/barback/web_socket_api.dart:226: throw throw new
json_rpc.RpcException.invalidParams(
I'm not a big fan of "invalid params" here; the error is coming from some
internal state, and I think of "invalid params" as referring to an error that is
detectable from looking at the parameters on their own. This feels like it
should have its own error code instead.

https://codereview.chromium.org/216593006/diff/1/sdk/lib/_internal/pub/lib/sr...
sdk/lib/_internal/pub/lib/src/barback/web_socket_api.dart:227: 'Path
"$rootDirectory" overlaps already served $dir $overlapping.');
It might be nice to add a [data] parameter here with the overlapping
directories.

https://codereview.chromium.org/216593006/diff/1/sdk/lib/_internal/pub/lib/sr...
File sdk/lib/_internal/pub/lib/src/command/barback.dart (right):

https://codereview.chromium.org/216593006/diff/1/sdk/lib/_internal/pub/lib/sr...
sdk/lib/_internal/pub/lib/src/command/barback.dart:118: var overlapping = new
Set<String>();
Nit: I don't like generic annotations in method bodies.

[Bob Nystrom, 2014-04-04 20:29:45]

Thanks!

https://codereview.chromium.org/216593006/diff/1/sdk/lib/_internal/pub/lib/sr...
File sdk/lib/_internal/pub/lib/src/barback/web_socket_api.dart (right):

https://codereview.chromium.org/216593006/diff/1/sdk/lib/_internal/pub/lib/sr...
sdk/lib/_internal/pub/lib/src/barback/web_socket_api.dart:226: throw throw new
json_rpc.RpcException.invalidParams(
On 2014/04/03 22:20:56, nweiz wrote:
> I'm not a big fan of "invalid params" here; the error is coming from some
> internal state, and I think of "invalid params" as referring to an error that
is
> detectable from looking at the parameters on their own. This feels like it
> should have its own error code instead.

Done. I wasn't sure if "invalid params" covered validation based on internal
state or not.

https://codereview.chromium.org/216593006/diff/1/sdk/lib/_internal/pub/lib/sr...
sdk/lib/_internal/pub/lib/src/barback/web_socket_api.dart:227: 'Path
"$rootDirectory" overlaps already served $dir $overlapping.');
On 2014/04/03 22:20:56, nweiz wrote:
> It might be nice to add a [data] parameter here with the overlapping
> directories.

Done.

https://codereview.chromium.org/216593006/diff/1/sdk/lib/_internal/pub/lib/sr...
File sdk/lib/_internal/pub/lib/src/command/barback.dart (right):

https://codereview.chromium.org/216593006/diff/1/sdk/lib/_internal/pub/lib/sr...
sdk/lib/_internal/pub/lib/src/command/barback.dart:118: var overlapping = new
Set<String>();
On 2014/04/03 22:20:56, nweiz wrote:
> Nit: I don't like generic annotations in method bodies.

Technically it's not an annotation, but done.

[nweiz, 2014-04-04 22:08:26]

Why are some tests deleted in the most recent patch set?

https://codereview.chromium.org/216593006/diff/20001/sdk/lib/_internal/pub/li...
File sdk/lib/_internal/pub/lib/src/barback/web_socket_api.dart (right):

https://codereview.chromium.org/216593006/diff/20001/sdk/lib/_internal/pub/li...
sdk/lib/_internal/pub/lib/src/barback/web_socket_api.dart:226: data:
error.overlappingDirectories);
I'd probably make the data a map so that extra information can be added later if
necessary.

[Bob Nystrom, 2014-04-04 22:23:40]

On 2014/04/04 22:08:26, nweiz wrote:
> Why are some tests deleted in the most recent patch set?

Those were testing how the web socket API handled overlapping directories. Now
it explicitly forbids it so they aren't valid (and failed when they tried to run
pub serve with overlapping dirs).

https://codereview.chromium.org/216593006/diff/20001/sdk/lib/_internal/pub/li...
File sdk/lib/_internal/pub/lib/src/barback/web_socket_api.dart (right):

https://codereview.chromium.org/216593006/diff/20001/sdk/lib/_internal/pub/li...
sdk/lib/_internal/pub/lib/src/barback/web_socket_api.dart:226: data:
error.overlappingDirectories);
On 2014/04/04 22:08:26, nweiz wrote:
> I'd probably make the data a map so that extra information can be added later
if
> necessary.

Done.

[Bob Nystrom, 2014-04-04 22:27:21]

Committed patchset #3 manually as r34750 (presubmit successful).