Skip to content

Commit b2664a9

Browse files
author
Jonathan Corbet
committed
jobserver: Split up the big try: block
The parsing of jobserver options is done in a massive try: block that hides problems and (perhaps) bugs. Split up that block and make the logic explicit by moving the initial parsing of MAKEFLAGS out of that block. Add warnings in the places things can go wrong. Reviewed-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> Signed-off-by: Jonathan Corbet <corbet@lwn.net>
1 parent 20f73d6 commit b2664a9

1 file changed

Lines changed: 90 additions & 53 deletions

File tree

‎tools/lib/python/jobserver.py‎

Lines changed: 90 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@
3535
import subprocess
3636
import sys
3737

38+
def warn(text, *args):
39+
print(f'WARNING: {text}', *args, file = sys.stderr)
40+
3841
class JobserverExec:
3942
"""
4043
Claim all slots from make using POSIX Jobserver.
@@ -58,64 +61,98 @@ def open(self):
5861

5962
if self.is_open:
6063
return
61-
62-
try:
63-
# Fetch the make environment options.
64-
flags = os.environ["MAKEFLAGS"]
65-
# Look for "--jobserver=R,W"
66-
# Note that GNU Make has used --jobserver-fds and --jobserver-auth
67-
# so this handles all of them.
68-
opts = [x for x in flags.split(" ") if x.startswith("--jobserver")]
69-
70-
# Parse out R,W file descriptor numbers and set them nonblocking.
71-
# If the MAKEFLAGS variable contains multiple instances of the
72-
# --jobserver-auth= option, the last one is relevant.
73-
fds = opts[-1].split("=", 1)[1]
74-
75-
# Starting with GNU Make 4.4, named pipes are used for reader
76-
# and writer.
77-
# Example argument: --jobserver-auth=fifo:/tmp/GMfifo8134
78-
_, _, path = fds.partition("fifo:")
79-
80-
if path:
64+
self.is_open = True # We only try once
65+
self.claim = None
66+
#
67+
# Check the make flags for "--jobserver=R,W"
68+
# Note that GNU Make has used --jobserver-fds and --jobserver-auth
69+
# so this handles all of them.
70+
#
71+
flags = os.environ.get('MAKEFLAGS', '')
72+
opts = [x for x in flags.split(" ") if x.startswith("--jobserver")]
73+
if not opts:
74+
return
75+
#
76+
# Separate out the provided file descriptors
77+
#
78+
split_opt = opts[-1].split('=', 1)
79+
if len(split_opt) != 2:
80+
warn('unparseable option:', opts[-1])
81+
return
82+
fds = split_opt[1]
83+
#
84+
# As of GNU Make 4.4, we'll be looking for a named pipe
85+
# identified as fifo:path
86+
#
87+
if fds.startswith('fifo:'):
88+
path = fds[len('fifo:'):]
89+
try:
8190
self.reader = os.open(path, os.O_RDONLY | os.O_NONBLOCK)
8291
self.writer = os.open(path, os.O_WRONLY)
83-
else:
84-
self.reader, self.writer = [int(x) for x in fds.split(",", 1)]
92+
except (OSError, IOError):
93+
warn('unable to open jobserver pipe', path)
94+
return
95+
#
96+
# Otherwise look for integer file-descriptor numbers.
97+
#
98+
else:
99+
split_fds = fds.split(',')
100+
if len(split_fds) != 2:
101+
warn('malformed jobserver file descriptors:', fds)
102+
return
103+
try:
104+
self.reader = int(split_fds[0])
105+
self.writer = int(split_fds[1])
106+
except ValueError:
107+
warn('non-integer jobserver file-descriptors:', fds)
108+
return
109+
try:
110+
#
85111
# Open a private copy of reader to avoid setting nonblocking
86112
# on an unexpecting process with the same reader fd.
87-
self.reader = os.open("/proc/self/fd/%d" % (self.reader),
113+
#
114+
self.reader = os.open(f"/proc/self/fd/{self.reader}",
88115
os.O_RDONLY | os.O_NONBLOCK)
89-
90-
# Read out as many jobserver slots as possible
91-
while True:
92-
try:
93-
slot = os.read(self.reader, 8)
94-
if not slot:
95-
# Clear self.jobs to prevent us from probably writing incorrect file.
96-
self.jobs = b""
97-
raise ValueError("unexpected empty token from jobserver fd, invalid '--jobserver-auth=' setting?")
98-
self.jobs += slot
99-
except (OSError, IOError) as e:
100-
if e.errno == errno.EWOULDBLOCK:
101-
# Stop at the end of the jobserver queue.
102-
break
103-
# If something went wrong, give back the jobs.
104-
if self.jobs:
105-
os.write(self.writer, self.jobs)
106-
raise e
107-
108-
# Add a bump for our caller's reserveration, since we're just going
109-
# to sit here blocked on our child.
110-
self.claim = len(self.jobs) + 1
111-
112-
except (KeyError, IndexError, ValueError, OSError, IOError) as e:
113-
print(f"jobserver: warning: {repr(e)}", file=sys.stderr)
114-
# Any missing environment strings or bad fds should result in just
115-
# not being parallel.
116-
self.claim = None
117-
118-
self.is_open = True
116+
except (IOError, OSError) as e:
117+
warn('Unable to reopen jobserver read-side pipe:', repr(e))
118+
return
119+
#
120+
# OK, we have the channel to the job server; read out as many jobserver
121+
# slots as possible.
122+
#
123+
while True:
124+
try:
125+
slot = os.read(self.reader, 8)
126+
if not slot:
127+
#
128+
# Something went wrong. Clear self.jobs to avoid writing
129+
# weirdness back to the jobserver and give up.
130+
self.jobs = b""
131+
warn("unexpected empty token from jobserver;"
132+
" possible invalid '--jobserver-auth=' setting")
133+
self.claim = None
134+
return
135+
except (OSError, IOError) as e:
136+
#
137+
# If there is nothing more to read then we are done.
138+
#
139+
if e.errno == errno.EWOULDBLOCK:
140+
break
141+
#
142+
# Anything else says that something went weird; give back
143+
# the jobs and give up.
144+
#
145+
if self.jobs:
146+
os.write(self.writer, self.jobs)
147+
self.claim = None
148+
warn('error reading from jobserver pipe', repr(e))
149+
return
150+
self.jobs += slot
151+
#
152+
# Add a bump for our caller's reserveration, since we're just going
153+
# to sit here blocked on our child.
154+
#
155+
self.claim = len(self.jobs) + 1
119156

120157
def close(self):
121158
"""Return all reserved slots to Jobserver"""

0 commit comments

Comments
 (0)