Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER
- Added user configurable setting of ninja depfile format via NINJA_DEPFILE_PARSE_FORMAT.
Now setting NINJA_DEPFILE_PARSE_FORMAT to [msvc,gcc,clang] can force the ninja expected
format. Compiler tools will also configure the variable automatically.
- Updated ninja scons daemon scripts to output errors to stderr as well as the daemon log.

From Mats Wichmann:
- Tweak the way default site_scons paths on Windows are expressed to
Expand Down
1 change: 1 addition & 0 deletions RELEASE.txt
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ IMPROVEMENTS
and msvc batch file determination when configuring the build environment. Simplify the msvc
code by eliminating special case handling primarily due to the differences between the full
versions and express versions of visual studio.
- Updated ninja scons daemon scripts to output errors to stderr as well as the daemon log.

PACKAGING
---------
Expand Down
10 changes: 7 additions & 3 deletions SCons/Tool/ninja/ninja_daemon_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@
level=logging.DEBUG,
)

def log_error(msg):
logging.debug(msg)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. if it's an error, isn't logging.error more appropriate?
  2. I know it's possible to configure logging to go both places, but if it needs to be log-level-dependent which goes where, I know it's not all that simple (multiple loggers, inheritance, etc. - may not be worth it). The simple case is to just add stream=sys.stderr to the initialization in basicConfig.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. The logging levels are useful when the volume of logging is overwhelming. Currently (IMHO) the ninja daemon doesn't put out enough logging for the logging levels to really be useful. Not saying logging.error is not more appropriate or correct, just kind of meaningless in this context and use case. We can easily change it in the future it the logging become hard to manage.

  2. Maybe one day if the logging situation calls for it. For now I think this is fine.

sys.stderr.write(msg)

while True:
try:
logging.debug(f"Sending request: {sys.argv[3]}")
Expand All @@ -68,19 +72,19 @@
except (http.client.RemoteDisconnected, http.client.ResponseNotReady):
time.sleep(0.01)
except http.client.HTTPException:
logging.debug(f"Error: {traceback.format_exc()}")
log_error(f"Error: {traceback.format_exc()}")
exit(1)
else:
msg = response.read()
status = response.status
if status != 200:
print(msg.decode("utf-8"))
log_error(msg.decode("utf-8"))
exit(1)
logging.debug(f"Request Done: {sys.argv[3]}")
exit(0)

except Exception:
logging.debug(f"Failed to send command: {traceback.format_exc()}")
log_error(f"Failed to send command: {traceback.format_exc()}")
exit(1)


Expand Down
17 changes: 9 additions & 8 deletions SCons/Tool/ninja/ninja_run_daemon.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@
level=logging.DEBUG,
)

def log_error(msg):
logging.debug(msg)
sys.stderr.write(msg)

if not os.path.exists(ninja_builddir / "scons_daemon_dirty"):
cmd = [
sys.executable,
Expand Down Expand Up @@ -107,14 +111,13 @@
except (http.client.RemoteDisconnected, http.client.ResponseNotReady):
time.sleep(0.01)
except http.client.HTTPException:
logging.debug(f"Error: {traceback.format_exc()}")
sys.stderr.write(error_msg)
log_error(f"Error: {traceback.format_exc()}")
exit(1)
else:
msg = response.read()
status = response.status
if status != 200:
print(msg.decode("utf-8"))
log_error(msg.decode("utf-8"))
exit(1)
logging.debug("Server Responded it was ready!")
break
Expand All @@ -123,15 +126,13 @@
logging.debug(f"Server not ready, server PID: {p.pid}")
time.sleep(1)
if p.poll is not None:
logging.debug(f"Server process died, aborting: {p.returncode}")
log_error(f"Server process died, aborting: {p.returncode}")
sys.exit(p.returncode)
except ConnectionResetError:
logging.debug("Server ConnectionResetError")
sys.stderr.write(error_msg)
log_error("Server ConnectionResetError")
exit(1)
except Exception:
logging.debug(f"Error: {traceback.format_exc()}")
sys.stderr.write(error_msg)
log_error(f"Error: {traceback.format_exc()}")
exit(1)

# Local Variables:
Expand Down