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

Make display detection more robust. #916

Merged
merged 6 commits into from
Jul 27, 2023
Merged

Make display detection more robust. #916

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5259b72
Refactor X11 display detection.
nuclearsandwich Apr 28, 2023
c1c95c0
Add a short wait to allow lightdm to start X11 after being restarted.
nuclearsandwich Apr 28, 2023
b11e09a
Remember to print the sed substitution.
nuclearsandwich Apr 28, 2023
1f266f0
Add : prefix to display value.
nuclearsandwich Apr 28, 2023
73ba462
Quote value to prevent shell misbehavior.
nuclearsandwich Jul 27, 2023
a2ff47e
Use systemctl directly instead of relying on wrapper.
nuclearsandwich Jul 27, 2023
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
25 changes: 15 additions & 10 deletions jenkins-scripts/lib/check_graphic_card.bash
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,19 @@ GRAPHIC_CARD_PKG=""

export_display_variable()
{
# Hack to found the current display (if available) two steps:
# Check for /tmp/.X11-unix/ socket and check if the process is running
for i in $(ls /tmp/.X11-unix/ | sed -e 's@^X@:@')
# Check for an active X11 display socket.
for i in $(find /tmp/.X11-unix -type s)
j-rivero marked this conversation as resolved.
Show resolved Hide resolved
do
# grep can fail so let's disable the fail or error during its call
set +e
ps aux | grep bin/X.*$i | grep -v grep
set -e
if [ $? -eq 0 ] ; then
export DISPLAY=$i
# If a process is running with the open socket then
# lsof will exit successfully.
if lsof -Fp $i ; then
j-rivero marked this conversation as resolved.
Show resolved Hide resolved
# Strip the path and leading X from the X11 socket
# but check that the resulting string is numeric and
# non-empty before exporting.
DISPLAY=:$(basename $i | sed -n -E 's/^X([0-9]+)/:\1/p')
Comment on lines +12 to +15
Copy link
Contributor Author

Choose a reason for hiding this comment

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

$i is no longer just the value of the DISPLAY variable so once confirming a likely active X11 server we need to get the value of DISPLAY for export. Compared with the prior substitution this one suppresses values that do not match X[0-9]+.

j-rivero marked this conversation as resolved.
Show resolved Hide resolved
if [ -n "$DISPLAY" ]; then
export DISPLAY
fi
fi
done
}
Expand Down Expand Up @@ -79,7 +82,9 @@ if $GPU_SUPPORT_NEEDED; then
if [[ ${DISPLAY} == "" ]]; then
echo "GPU support needed by the script but DISPLAY var is empty"
# Try to restart lightdm. It should stop the script in the case of failure
sudo service lightdm restart
sudo systemctl restart lightdm
# Wait for lightdm service to restart X11.
sleep 5
Comment on lines +86 to +87
Copy link
Contributor Author

Choose a reason for hiding this comment

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

5 seconds is possibly way too long, but if this does not work as expected we likely need a different hypothesis.

Copy link
Contributor

Choose a reason for hiding this comment

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

Trying to figure out how we can inspect systemd activations, maybe a loop with a timeout on systemctl is-active can work here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I'm not certain that the watching lightdm activation / status would be sufficient to resolve the race since I don't know if lightdm uses systemd's notification API for actually declaring readiness or if it's a simple/exec type service which is considered active as soon as the process runs https://www.freedesktop.org/software/systemd/man/systemd.service.html in which case we'd still need to wait ourselves for X11 to start.

# Second try to get display variable
export_display_variable
if [[ ${DISPLAY} == "" ]]; then
Expand Down