Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bazel.exe should not need msys-2.0.dll to run #2107

Closed
dslomov opened this issue Nov 18, 2016 · 9 comments
Closed

bazel.exe should not need msys-2.0.dll to run #2107

dslomov opened this issue Nov 18, 2016 · 9 comments
Assignees
@laszlocsomor
Labels
P1 I'll work on this now. (Assignee required) platform: windows type: feature request
Milestone
0.5

Comments

@dslomov
Copy link
Contributor

dslomov commented Nov 18, 2016

Umbrella bug for removing msys usage from the launcher
@dslomov dslomov added platform: windows P1 I'll work on this now. (Assignee required) type: feature request labels Nov 18, 2016
@dslomov dslomov added this to the 0.5 milestone Nov 18, 2016
@dslomov dslomov mentioned this issue Nov 18, 2016
Closed
@laszlocsomor
Copy link
Contributor

Commits pertaining to this bug so far: b9d7767, f926f3e, a85f52d, 943d3cf, 6c16765, 8a48f61, 6bf9576, 251bf03, 9c95196

bazel-io pushed a commit that referenced this issue Nov 22, 2016
@laszlocsomor @dslomov
Bazel client: reduce dependency on POSIX API
cefa9a2 
We can now compile blaze_util_windows.cc with
MSVC, yay! (when building //src:bazel
--cpu=x64_windows_msvc -k).

There are a lot of #ifdef's and TODOs so this
is a modest victory for now.

In this change:

- change blaze::MakeDirectories to return bool
instead of int, since that's how it was used
anyway, and to expect the permission mask as
unsigned int instead of mode_t, since the
former is good enough and compatible with
mode_t on POSIX while mode_t is not defined on
Windows

- move blaze::MakeDirectories into
blaze_util_<platform>

- implement envvar-handling in
blaze_util_<platform> and use it everywhere

See #2107

--
MOS_MIGRATED_REVID=139887503
bazel-io pushed a commit that referenced this issue Nov 24, 2016
@laszlocsomor @dslomov
Bazel client: delete and don't use the server.pid
6450c18 
Upon startup the Bazel client checks if there's
already a running server process and if so then
connects to it.

We achieve this by checking if there's a symlink
in the server directory called served.pid,
pointing to /proc/<server_pid>. If so, we read the
symlink's target and extract the PID; otherwise we
check if there's a file in the server's directory
(server.pid.txt) that contains the PID and read it
from there.

Since the PID file is always there, we don't need
the symlink, plus on Windows we don't support
symlinks anyway, which is the real motivation for
this change.

Just ignoring the PID symlink is not enough, we
need to actively delete it so that switching
between Bazel versions (one that writes a PID
symlink and one that doesn't) won't result in
having a symlink and PID file with different
PIDs and clients trying to kill the wrong server
process / not killing one that they should.

See #2107

--
MOS_MIGRATED_REVID=140117287
bazel-io pushed a commit that referenced this issue Nov 24, 2016
@laszlocsomor @dslomov
Bazel client: depend less on <unistd.h>
74ffaf7 
We moved most of the functionality (e.g. _exit,
SetupStreams) into blaze_util_<platform> or
changed to alternative functions (fwrite + stderr
instead of write + STDERR_HANDLE).

This change brings us closer to compiling blaze.cc
with MSVC. We still have to move signal handlers
out of blaze.cc as well as code dealing with the
server PID.

See #2107

--
MOS_MIGRATED_REVID=140123945
bazel-io pushed a commit that referenced this issue Nov 24, 2016
@laszlocsomor @dslomov
Bazel client: MakeCanonical into file_<platform>
c354539 
Move MakeCanonical into platform-specific files.
Also change the signature of
blaze.cc:CheckBinaryPath to return the binary path
instead of mutating `globals`.

See #2107

--
MOS_MIGRATED_REVID=140128173
bazel-io pushed a commit that referenced this issue Nov 24, 2016
@laszlocsomor @dslomov
Bazel client: fix compilation on Windows
8dddb3c 
Add "#include <fcntl.h>".

See #2107

--
MOS_MIGRATED_REVID=140131127
bazel-io pushed a commit that referenced this issue Nov 24, 2016
@laszlocsomor @dslomov
Bazel client: platform-dependent signal handling
32086b2 
Move the signal handling code from blaze.cc into
blaze_util_<platform>.

See #2107

--
MOS_MIGRATED_REVID=140134781
bazel-io pushed a commit that referenced this issue Nov 28, 2016
@laszlocsomor @iirina
Bazel client: mock out read/write calls
26b7a66 
Make blaze::ReadFileDescriptor(int fd, ...) and
blaze::WriteFile(int fd, ...) platform-independent
by mocking out the read(2) and write(2) calls.

Also rename ReadFileDescriptor to ReadFrom and
introduce a new WriteTo method that encapsulates
WriteFile's prior logic.

In particular these functions now take a
read_func/write_func function argument instead of
a file descriptor, so the read(2)/write(2) calls
can be mocked out.

This allows us to use these functions on Windows
too, where read(2)/write(2) are not implemented,
and we can inject a different
read_func/write_func.

See #2107

--
MOS_MIGRATED_REVID=140195973
bazel-io pushed a commit that referenced this issue Nov 28, 2016
@laszlocsomor @iirina
Bazel client: platform-dependent lock handling
7e5fb40 
Move blaze::AcquireLock and blaze::ReleaseLock
into blaze_util_<platform>.

See #2107

--
MOS_MIGRATED_REVID=140200355
bazel-io pushed a commit that referenced this issue Nov 28, 2016
@laszlocsomor @iirina
Bazel client: platform-specific {Read,Write}File
49970e0 
Move blaze::ReadFile and blaze::WriteFile to
file.h and file_platform.h (thus into the
blaze_util namespace), and update references.

This allows us to implement these methods in a
platform-specific way.

Also move UnlinkPath.

See #2107

--
MOS_MIGRATED_REVID=140328273
bazel-io pushed a commit that referenced this issue Nov 28, 2016
@laszlocsomor @aehlig
Bazel client: more platform-specific logic
d0a1269 
Move terminal-querying functions and GetUserName
from blaze_util.cc into blaze_util_<platform>.

See #2107

--
MOS_MIGRATED_REVID=140346402
bazel-io pushed a commit that referenced this issue Nov 28, 2016
@laszlocsomor @aehlig
Bazel client: make it compile with MSVC
3b89d2d 
Final modifications to the Bazel client code so
we can compile //src/main/cpp/...:all using MSVC.
Yay!

We still have some dependencies that don't compile
with MSVC, namely Ijar, build-runfiles,
process-wrapper, and process-tools.

Still, this change is a huge success, because now
we can add regression tests to prevent people from
introducing breaking changes to the client that
would break Windows/MSVC compilation.

It's important to point out that we can only build
this library for now, most functions in
file_windows.cc and blaze_util_windows.cc have an
empty body (they just call `pdie`).

See #2107

--
MOS_MIGRATED_REVID=140348351
@laszlocsomor laszlocsomor mentioned this issue Dec 1, 2016
Closed
bazel-io pushed a commit that referenced this issue Dec 1, 2016
@laszlocsomor @iirina
Ijar: extract file stating logic to platform_utils
645dbc4 
This change takes us closer to compiling ijar,
thus Bazel, with MSVC.

See #2107 and #2157

--
MOS_MIGRATED_REVID=140717828
bazel-io pushed a commit that referenced this issue Dec 1, 2016
@laszlocsomor @iirina
Ijar: extract file writing logic to platform_utils
8d6da00 
This change takes us closer to compiling ijar,
thus Bazel, with MSVC.

Also update the StatFile method added by
unknown commit to report any errors.

See #2157
See #2107

--
MOS_MIGRATED_REVID=140719249
bazel-io pushed a commit that referenced this issue Dec 1, 2016
@laszlocsomor @iirina
Ijar: extract file reading logic to platform_utils
d2ed069 
This change takes us closer to compiling ijar,
thus Bazel, with MSVC.

See #2157
See #2107

--
MOS_MIGRATED_REVID=140722341
bazel-io pushed a commit that referenced this issue Dec 1, 2016
@laszlocsomor @iirina
Ijar: extract [] to platform_utils
8457f3f 
zip_main.cc no longer needs <unistd.h>.
This change takes us closer to compiling ijar,
thus Bazel, with MSVC.

See #2157
See #2107

--
MOS_MIGRATED_REVID=140723658
bazel-io pushed a commit that referenced this issue Dec 1, 2016
@laszlocsomor @iirina
Ijar: extract MakeDirs to platform_utils
b8caca0 
zip_main.cc no longer needs <unistd.h>.
This change takes us closer to compiling ijar,
thus Bazel, with MSVC.

See #2157
See #2107

--
MOS_MIGRATED_REVID=140724421
bazel-io pushed a commit that referenced this issue Dec 2, 2016
@laszlocsomor @kchodorow
Ijar: can be compiled with MSVC
479e18d 
We can now build //third_party/ijar/...:all with
--cpu=x64_windows_msvc.

We also have to use --output_user_root=/c/tmp
or something similarly short because
//third_party/zlib tickles #2145

This change takes us closer to compiling Bazel
with MSVC.

See #2107

--
PiperOrigin-RevId: 140846600
MOS_MIGRATED_REVID=140846600
@laszlocsomor laszlocsomor mentioned this issue Dec 9, 2016
Closed
@damienmg
Copy link
Contributor

Is this going to happen by end of next month?

@laszlocsomor
Copy link
Contributor

Yes, I think end of January is a realistic goal.

@petemounce
Copy link
Contributor

By the way, 0.4.2 cleared the bazel moderation queue fine.

bazel-io pushed a commit that referenced this issue Dec 15, 2016
@laszlocsomor @katre
Bazel client, Windows: implement GetUserName
41ee591 
See #2107

--
PiperOrigin-RevId: 142128121
MOS_MIGRATED_REVID=142128121
bazel-io pushed a commit that referenced this issue Feb 7, 2017
@laszlocsomor @kchodorow
Bazel client, Windows: compile more with MSVC
5eb3ef9 
In this change:
* remove some pdie calls from unimplemented method
bodies in blaze_util_windows.cc, if implementing
that method is not that important and/or can wait
* move code from the msys-only #ifdef parts to
common where it makes sense

See #2107

--
PiperOrigin-RevId: 146791575
MOS_MIGRATED_REVID=146791575
bazel-io pushed a commit that referenced this issue Feb 8, 2017
@laszlocsomor @kchodorow
Bazel client: retry moving install base directory
918814e 
Do not give up immediately if renaming/moving the
install base directory fails, wait and retry
instead.

This is necessary because on Windows the directory
we just created and populated with the extracted
embedded binaries may still be scanned by the
antivirus, so there are open file handles in it so
it cannot be renamed. This new logic ensures the
AV has enough time to scan all files, close the
handles, letting us successfully rename the
directory.

Fixes the occasional "install base directory
cannot be renamed in place" error messages on
Windows.

See #2107

--
PiperOrigin-RevId: 146899919
MOS_MIGRATED_REVID=146899919
bazel-io pushed a commit that referenced this issue Feb 9, 2017
@laszlocsomor @kchodorow
Bazel client, Windows: implement SetupStdStreams
26f858c 
See #2107

--
PiperOrigin-RevId: 146921464
MOS_MIGRATED_REVID=146921464
bazel-io pushed a commit that referenced this issue Feb 14, 2017
@laszlocsomor @dslomov
Windows JNI, refactor: move OpenDirectory to JNI
5efe4a1 
Move the OpenDirectory helper method into the JNI
library. We'll need it there; a subsequent change
will make use of it there.

See #2107

--
PiperOrigin-RevId: 147448792
MOS_MIGRATED_REVID=147448792
bazel-io pushed a commit that referenced this issue Feb 15, 2017
@laszlocsomor @dslomov
Bazel client, JNI, Windows: impl. CreateJunction
d7d4def 
Implement a CreateJunction function in the Windows
JNI library. Also move a bit of code from
file_windows to the JNI library, where it is
(also) needed.

This implementation is an improved version of
`blaze_util::SymlinkDirectories` in
blaze_util_windows: this version handles Windows
paths as `name` and `target`, and performs more
validation (e.g. on the length of `target`), plus
has more comments explaining the logic. In a
subsequent change I'll start using this new
function in blaze_util_windows.

This method will also be helpful in tests: we will
no longer have to shell out to mklink.

See #2107

--
Change-Id: I7e9b085fdc2ba47be83da5319bded02bd323e71b
Reviewed-on: https://cr.bazel.build/8892
PiperOrigin-RevId: 147585207
MOS_MIGRATED_REVID=147585207
bazel-io pushed a commit that referenced this issue Feb 15, 2017
@laszlocsomor @dslomov
Bazel client, Windows: use CreateJunction
38db316 
Use the JNI library's CreateJunction function in
blaze_util_windows, and delete the old code.

See #2107

--
Change-Id: I1cb027a7c7ceb559e590e89e93f9022607fc0cd5
Reviewed-on: https://cr.bazel.build/8893
PiperOrigin-RevId: 147587975
MOS_MIGRATED_REVID=147587975
bazel-io pushed a commit that referenced this issue Feb 15, 2017
@laszlocsomor @dslomov
Bazel client, Windows, tests: remove cleanup code
ce74be1 
The TearDown method clears up after tests, so
manual cleanup is unnecessary.

See #2107

--
Change-Id: Idf5d2b2bf012774171f1868d1341a7952015c35f
Reviewed-on: https://cr.bazel.build/8894
PiperOrigin-RevId: 147591032
MOS_MIGRATED_REVID=147591032
bazel-io pushed a commit that referenced this issue Feb 15, 2017
@laszlocsomor @hermione521
Bazel client, Windows, tests: use CreateJuction
3635539 
Use the JNI library's CreateJuction in
file_windows_test.

See #2107

--
Change-Id: I4ef1536d43691fe7a2ae3ee457064d4e8f4ac6d7
Reviewed-on: https://cr.bazel.build/8895
PiperOrigin-RevId: 147594365
MOS_MIGRATED_REVID=147594365
bazel-io pushed a commit that referenced this issue Feb 16, 2017
@laszlocsomor @dslomov
Bazel client, Windows: implement ConvertPath
a3ab098 
See #2107

--
Change-Id: I7a325521f52bc4027b55df5451f549ac737242f8
Reviewed-on: https://cr.bazel.build/8953
PiperOrigin-RevId: 147715340
MOS_MIGRATED_REVID=147715340
bazel-io pushed a commit that referenced this issue Feb 16, 2017
@laszlocsomor @dslomov
Bazel client, Windows: implement MakeCanonical
9b75b68 
See #2107

--
Change-Id: I27a97881e3e19cbb7913e1248a24e9e631bc4f40
Reviewed-on: https://cr.bazel.build/8951
PiperOrigin-RevId: 147719277
MOS_MIGRATED_REVID=147719277
bazel-io pushed a commit that referenced this issue Feb 17, 2017
@laszlocsomor @dslomov
Bazel client, Windows: impl. ForEachDirectoryEntry
748f8af 
This is the last function we needed from
file_posix for MSYS, so now we can remove that
from the compilation.

Fixes #2386
The problem originally was that I used CloseHandle
to close the HANDLE, instead of using FindClose,
so we were holding on to open directory HANDLEs,
so we couldn't rename the installation directory.

See #2107

--
Change-Id: If6c1b3c99cf386d82e829dbee2323e6bce5f6b46
Reviewed-on: https://cr.bazel.build/8952
PiperOrigin-RevId: 147734165
MOS_MIGRATED_REVID=147734165
bazel-io pushed a commit that referenced this issue Feb 28, 2017
@laszlocsomor @hermione521
MSVC wrapper: support -I gcc flag
1409aa3 
See #2107

--
PiperOrigin-RevId: 148750327
MOS_MIGRATED_REVID=148750327
bazel-io pushed a commit that referenced this issue Mar 6, 2017
@laszlocsomor @vladmos
Bazel client, Windows: fix AsShortWindowsPath
2b01b5d 
This method now works for non-existent paths too.

See #2107

--
PiperOrigin-RevId: 149284633
MOS_MIGRATED_REVID=149284633
bazel-io pushed a commit that referenced this issue Mar 6, 2017
@laszlocsomor @vladmos
Bazel client, Windows: fix ReadFile("/dev/null")
e6d40b3 
Mimic read(2) behavior when reading from /dev/null
and always successfully read 0 bytes.

See #2107

--
PiperOrigin-RevId: 149286494
MOS_MIGRATED_REVID=149286494
bazel-io pushed a commit that referenced this issue Mar 6, 2017
@laszlocsomor @vladmos
Windows, JNI: shorten the cwd for CreateProcess
373879d 
The hard limit for SetCurrentDirectory{A,W} is
MAX_PATH-1, even with UNC prefix, therefore a
process' cwd may also not be longer than that.

See #2107
See #2406
See #2181

--
PiperOrigin-RevId: 149290147
MOS_MIGRATED_REVID=149290147
bazel-io pushed a commit that referenced this issue Mar 7, 2017
@laszlocsomor @vladmos
Bazel client, Windows/MSVC: fix path handling bugs
9d3f989 
Fix blaze_util_windows.ConvertPath: in the MSVC
version this is using the actual %PATH% value, we
don't need to convert it.

Fix blaze_util_windows.PathAsJvmFlag: shorten the
path so we can pass it to the JVM process (long
paths aren't understood by the shell), but also
converrt backslashes to forward slashes so the JVM
won't believe we are passing paths with escaped
characters.

See #2107
See #2181

--
PiperOrigin-RevId: 149396971
MOS_MIGRATED_REVID=149396971
bazel-io pushed a commit that referenced this issue Mar 9, 2017
@laszlocsomor @vladmos
Bazel client: use PathAsJvmFlag for JVM flags
4875b25 
The Windows-implementation of this function
shortens paths that we pass to the JVM, such as
the Bazel server jar's path, the log file path,
etc. These must be shortened because the JVM
doesn't handle long paths.

We don't shorten paths passed to the Bazel server
itself though because Bazel can handle long paths.

See #2107

--
PiperOrigin-RevId: 149548361
MOS_MIGRATED_REVID=149548361
bazel-io pushed a commit that referenced this issue Mar 9, 2017
@laszlocsomor @vladmos
Singlejar: Windows-aware jar tool path
561e449 
See #2107

--
PiperOrigin-RevId: 149626394
MOS_MIGRATED_REVID=149626394
bazel-io pushed a commit that referenced this issue Mar 9, 2017
@laszlocsomor @vladmos
Bazel client, Windows: CreateDirectoryW path limit
52ab7d9 
Fix the path limit for non-UNC-prefixed paths when
using CreateDirectoryW. According to MSDN [1],
this is only 248 chars, as opposed to the usual
260 (MAX_PATH).

See #2107

[1] https://msdn.microsoft.com/en-us/library/windows/desktop/aa363855(v=vs.85).aspx

--
PiperOrigin-RevId: 149627964
MOS_MIGRATED_REVID=149627964
bazel-io pushed a commit that referenced this issue Mar 14, 2017
@laszlocsomor @meteorcloudy
Bazel client, Windows: make server mode work
c8cd6bd 
Fix blaze_util_windows::ConvertAbsolutePaths,
which happens to have been the culprit why the
MSYS-less Bazel would always kill the running
server.

Fixes #2672
See #2107

--
Change-Id: I873a78c737a6d6906ac7db9bcd0e7186e17bd7ca
Reviewed-on: https://cr.bazel.build/9355
PiperOrigin-RevId: 150052180
MOS_MIGRATED_REVID=150052180
bazel-io pushed a commit that referenced this issue Mar 14, 2017
@laszlocsomor @meteorcloudy
Bazel client, Windows: can connect to fresh server
5fa18d1 
Fix the bug that the MSYS-less client couldn't
connect to the freshly started server and had to
be started again.

Fixes #2672
See #2107

--
PiperOrigin-RevId: 150069285
MOS_MIGRATED_REVID=150069285
@laszlocsomor laszlocsomor mentioned this issue Mar 15, 2017
Closed
bazel-io pushed a commit that referenced this issue Mar 16, 2017
@laszlocsomor @meteorcloudy
Bazel client, Windows: fix console handling bugs
3d97f49 
Fix 3 bugs in blaze_util_windows:
- off-by-one error in the loop ensuring the std
handles are all open
- don't auto-close stdout's HANDLE after querying
the console window's size
- error-handling for when stdout is redirected
thus ::GetStdHandle returns an invalid handle

See #2107

--
PiperOrigin-RevId: 150310578
MOS_MIGRATED_REVID=150310578
bazel-io pushed a commit that referenced this issue Mar 16, 2017
@laszlocsomor @meteorcloudy
Bazel client, Windows: support colorful output
ae4b02f 
See #2107

--
PiperOrigin-RevId: 150314355
MOS_MIGRATED_REVID=150314355
bazel-io pushed a commit that referenced this issue Mar 22, 2017
@laszlocsomor
Windows, protoc: create io_win32.{h,cc}
184f3c8 
Create dedicated files for the long-path-aware
Windows implementations of open/access/mkdir.

This commit updates many BUT NOT ALL usages of
<io.h> functions in protobuf's code base. Reason
being is that there are no Bazel build rules for
the unittest files that include <io.h>, so I
decided to leave those alone.

Thanks to this commit I can now build Bazel with
MSVC without needing a short --output_user_base.

Fixes #2634
See #2107
See protocolbuffers/protobuf#2891

Change-Id: I374726452300854a36e4628bb22cb7bbb12f3bad
@laszlocsomor
Copy link
Contributor

It's my greatest pleasure to close this bug.

With 0.5.0 we'll start dogfooding the MSYS-less version, i.e. a Bazel binary built with MSVC and not depending on msys-2.0.dll. Bazel will still require an MSYS bash installation (we're also working on supporting other Bashes like Cygwin, see #2725) to build genrule and sh_* rules, but we will no longer require a particular version of it.

@dslomov
Copy link
Contributor Author

dslomov commented Apr 4, 2017

Woo-hoo!

@damienmg
Copy link
Contributor

damienmg commented Apr 4, 2017 via email

bazel-io pushed a commit that referenced this issue Apr 13, 2017
@laszlocsomor @buchgr
Bazel client, Windows: impl. client-level locking
ade79a7 
Implement blaze::AcquireLock and ReleaseLock.

These methods implement the Bazel client-level
locking, whose purpose is to detect concurrently
running Bazel instances attempting to write to the
same output directory.

The Bazel server also detects this case (see
BlazeCommandDispatcher) but the client needs to
start the server first, meaning this cannot detect
races between clients that are in the middle of
installing.

You can see this locking in effect if you run
`bazel --output_user_root=/c/foo build src:bazel`
in one terminal, then run
`bazel --output_user_root=/c/foo help` in another
but the same working directory. The
second one will say "Another command is running."

See #2107
See #2647

RELNOTES: none
PiperOrigin-RevId: 152919185
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@laszlocsomor laszlocsomor

Labels
P1 I'll work on this now. (Assignee required) platform: windows type: feature request
Projects
None yet
Milestone
0.5
Development

No branches or pull requests
4 participants
@petemounce @damienmg @dslomov @laszlocsomor