buildbarn / bb-storage

Storage daemon, capable of storing data for the Remote Execution protocol
Apache License 2.0
142 stars 91 forks source link

Windows Port for local_directory #126

Closed mou-hao closed 3 years ago

mou-hao commented 3 years ago

Hi,

This change intends to port local_directory to Windows and is part of buildbarn/bb-remote-execution#7. I want to make sure this is the right approach before I go ahead and implement a Windows worker.

I saw some previous PRs related to this issue and understand there is a need for a file descriptor based approach to avoid race conditions and to have multiple conceptual cwd's, so in my port, local_directory is totally windows file handle based.

The implementation passes all existing tests (except for IsWritable and IsWritableChild because of the very different file access control model).

There are some caveats though:

Cheers, mh

I am requested by my employer to include the following disclaimer here:

THIS SOFTWARE IS CONTRIBUTED SUBJECT TO THE TERMS OF THE APACHE LICENSE V.2.0. YOU MAY OBTAIN A COPY OF THE LICENSE AT https://www.apache.org/licenses/LICENSE-2.0. THIS SOFTWARE IS LICENSED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE AND ANY WARRANTY OF NON-INFRINGEMENT, ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. THIS SOFTWARE MAY BE REDISTRIBUTED TO OTHERS ONLY BY EFFECTIVELY USING THIS OR ANOTHER EQUIVALENT DISCLAIMER IN ADDITION TO ANY OTHER REQUIRED LICENSE TERMS.

mou-hao commented 3 years ago

I created a PR to x/sys/windows to add the constants: https://go-review.googlesource.com/c/sys/+/355350 Also created a proposal to add NtSetInformationFile: https://github.com/golang/go/issues/48933 Added them in a comment in the build file of windowsext, so we won't forget about them.

This should be good for review now. Cheers.