Summary:
This prevents `hg status` from blowing up with a UTF-8 decode
error inside the generated thrift code.
Push safety concerns:
* This doesn't change the wire representation of the data
* Existing clients that believe it to be a string will continue to have
the same behavior
* Buck has its own copy of an older version of the thrift spec, so it will
continue to work "OK".
* When buck resyncs with our thrift file, some changes will likely be needed
to convert the byte arrays to strings or paths or whatever is appropriate
for bucks internal API
Work "OK" above means that clients that currently believe that `string` is
utf-8 encoded will have a runtime error if we ever send them a path that
is not utf-8. This is the behavior prior to this diff and will continue
to be the behavior for clients (like buck) that have an older version
of the thrift file.
Reviewed By: simpkins
Differential Revision: D9270843
fbshipit-source-id: b01135aec9152aaf5199e1c654ddd7f61c03717e
Summary:
This makes it easier to add some test coverage.
There's no real functional change in this diff; the only code change is to
throw a system_error instead of a thrift eden error wrapper class from the core
globbing code. There's a little bit of code to restore this exception type in
the callers in EdenServiceHandler; this is covered by existing integration
tests, but I've also expanded that coverage to cover both variants of the glob
thrift calls.
Reviewed By: strager
Differential Revision: D8776767
fbshipit-source-id: 3ea4ea642ae5108aa4b0153541bd3604f010b54c
Summary: Mostly empty lines removed and added. A few bugfixes on excessive line splitting.
Reviewed By: cooperlees
Differential Revision: D8198776
fbshipit-source-id: 4361faf4a2b9347d57fb6e1342c494575f2beb67
Summary:
We need to introduce a new `includeDotfiles` option to `glob()`. [As we have
done for all of our Thrift API, to date], rather than define `glob()` so that it
takes a single struct, we specified the parameters individually, so we can no
longer add new params to `glob()`.
In particular, we need to support `includeDotfiles` because we often configure
Buck to use Watchman to implement `glob()` in `BUCK` files, and when Watchman is
used in Eden, it leverages Eden's Thrift API to implement `glob()`. Because
Buck's `glob()` has an `include_dotfiles` option, we must be able to honor it
and pass it all the way through to Eden's `glob()` implementation.
Rather than name the new API `glob2()`, I'm electing to go with `globFiles()`.
(Perhaps once we eliminate all known users of `glob()` in the wild, which
requires turning over the current version of Watchman we have deployed, we can
redefine `glob()` in `eden.thrift` to be the same as `globFiles()` and then
update everyone to use `glob()` again so it has the more intuitive name.)
Reviewed By: wez
Differential Revision: D7748870
fbshipit-source-id: 92438f9c41e4fbdbd6cdccca5fce0e41cc3e9b07
Summary:
By introducing `_assert_glob()`, it eliminates a bit of the boilerplate in the
individual test cases. I think it also makes things easier to read because now
the glob patterns appear before the result of the glob.
Though most importantly, this will help with a subsequent change where I am
going to change the contract of the Thrift API, as now it can be done within
`_assert_glob()`.
Reviewed By: wez
Differential Revision: D7748871
fbshipit-source-id: 9609cde104979e892f0858e7a0c7e53e976ff8e2
Summary:
In the spirit of TDD, I am writing the tests first and annotating them with
`unittest.skip()` so the build still succeeds.
Reviewed By: chadaustin
Differential Revision: D7741507
fbshipit-source-id: 4ede0b933c75f9be4016e399936ff2e47eb4e538
Summary:
I think this makes it easier to reason about the coverage of the space of
inputs to `glob()`.
Reviewed By: chadaustin
Differential Revision: D7741509
fbshipit-source-id: 5882d859df95279189512716004263dd5320ff3f
Summary:
I would like to to test more inputs for `glob()` with different characteristics.
I think this would be more logically organized when divided across a number of
test methods in a single `GlobTest` class.
This revision does a straight move of the eixsting `test_glob()` method without
introducing any new test cases.
Reviewed By: chadaustin
Differential Revision: D7741506
fbshipit-source-id: 141341d74265f3949ed7523f40a56f98d95ee13e
