Skip to content

playwright: fix firefox #197899

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

Closed
DGollings wants to merge 1 commit into from
Closed

playwright: fix firefox #197899

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
62 changes: 58 additions & 4 deletions pkgs/development/python-modules/playwright/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@
, buildPythonPackage
, chromium
, ffmpeg
, firefox
, firefox-bin
, git
, greenlet
, jq
, nodejs
, fetchFromGitHub
, fetchurl
, fetchzip
, makeFontsConf
, makeWrapper
, pyee
Expand All @@ -21,7 +22,10 @@
}:

let
inherit (builtins) fromJSON readFile listToAttrs;

@SuperSandro2000 SuperSandro2000 Feb 17, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
inherit (builtins) fromJSON readFile listToAttrs;

inherit (stdenv.hostPlatform) system;
selectSystem = attrs:
attrs.${system} or (throw "Unsupported system: ${system}");
Comment on lines +27 to +28

@SuperSandro2000 SuperSandro2000 Feb 17, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
selectSystem = attrs:
attrs.${system} or (throw "Unsupported system: ${system}");
selectSystem = attrs: attrs.${system} or (throw "Unsupported system: ${system}");

throwSystem = throw "Unsupported system: ${system}";

driverVersion = "1.27.1";
Expand Down Expand Up @@ -79,6 +83,22 @@ let
};
};

browser_revs =
let
file = fetchurl {
url =
"https://raw.githubusercontent.com/microsoft/playwright/v${driverVersion}/packages/playwright-core/browsers.json";
Comment on lines +89 to +90

@SuperSandro2000 SuperSandro2000 Feb 17, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
url =
"https://raw.githubusercontent.com/microsoft/playwright/v${driverVersion}/packages/playwright-core/browsers.json";
url = "https://raw.githubusercontent.com/microsoft/playwright/v${driverVersion}/packages/playwright-core/browsers.json";

sha256 = "11a1n65l43nfyjn0qrsjjyjl7psvqa67k4kiv8x624faga4zl3mk";
};
raw_data = fromJSON (readFile file);

@SuperSandro2000 SuperSandro2000 Feb 17, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
raw_data = fromJSON (readFile file);
raw_data = lib.importJSON file;

in
listToAttrs (map
({ name, revision, ... }: {
inherit name;
value = revision;
})
raw_data.browsers);
Comment on lines +95 to +100

@SuperSandro2000 SuperSandro2000 Feb 17, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
listToAttrs (map
({ name, revision, ... }: {
inherit name;
value = revision;
})
raw_data.browsers);
listToAttrs (map (
{ name, revision, ... }: {
inherit name;
value = revision;
}
) raw_data.browsers);


browsers-mac = stdenv.mkDerivation {
pname = "playwright-browsers";
version = driverVersion;
Expand All @@ -105,6 +125,20 @@ let
fontconfig = makeFontsConf {
fontDirectories = [];
};
suffix = selectSystem {
# Not sure how other system compatibility is, needs trial & error

@SuperSandro2000 SuperSandro2000 Feb 17, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
# Not sure how other system compatibility is, needs trial & error

thats fine :)

x86_64-linux = "ubuntu-20.04";
aarch64-linux = "ubuntu-20.04-arm64";
x86_64-darwin = "mac";
aarch64-darwin = "mac-arm64";
};

upstream_firefox = fetchzip {
url =
"https://playwright.azureedge.net/builds/firefox/${browser_revs.firefox}/firefox-${suffix}.zip";
Comment on lines +137 to +138

@SuperSandro2000 SuperSandro2000 Feb 17, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
url =
"https://playwright.azureedge.net/builds/firefox/${browser_revs.firefox}/firefox-${suffix}.zip";
url = "https://playwright.azureedge.net/builds/firefox/${browser_revs.firefox}/firefox-${suffix}.zip";

sha256 = "sha256-r1cCXxkIABU5BLP1Q6TBoE5CrNhmHApAEZT3ta+GaAU=";
stripRoot = true;
};
in runCommand ("playwright-browsers"
+ lib.optionalString (withFirefox && !withChromium) "-firefox"
+ lib.optionalString (!withFirefox && withChromium) "-chromium")
Expand All @@ -125,9 +159,29 @@ let
--set SSL_CERT_FILE /etc/ssl/certs/ca-bundle.crt \
--set FONTCONFIG_FILE ${fontconfig}
'' + lib.optionalString withFirefox ''
FIREFOX_REVISION=$(jq -r '.browsers[] | select(.name == "firefox").revision' $BROWSERS_JSON)
mkdir -p $out/firefox-$FIREFOX_REVISION/firefox
ln -s ${firefox}/bin/firefox $out/firefox-$FIREFOX_REVISION/firefox/firefox
firefoxoutdir=$out/firefox-${browser_revs.firefox}/firefox
mkdir -p $firefoxoutdir
cp -r ${upstream_firefox}/* $firefoxoutdir/

@SuperSandro2000 SuperSandro2000 Feb 17, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should think about symlinks or xorg.lndir


# patchelf the binary
wrapper="${firefox-bin}/bin/firefox"
binary="$(readlink -f $(<"$wrapper" grep '^exec ' | grep -o -P '/nix/store/[^"]+' | head -n 1))"

@SuperSandro2000 SuperSandro2000 Feb 17, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is at a fixed location? Or can we get that from the firefox attr via passthru?


interpreter="$(patchelf --print-interpreter "$binary")"
rpath="$(patchelf --print-rpath "$binary")"

find $firefoxoutdir/ -executable -type f | while read i; do
chmod u+w "$i"
[[ $i == *.so ]] || patchelf --set-interpreter "$interpreter" "$i"
patchelf --set-rpath "$rpath" "$i"
Comment on lines +170 to +176

@SuperSandro2000 SuperSandro2000 Feb 17, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
interpreter="$(patchelf --print-interpreter "$binary")"
rpath="$(patchelf --print-rpath "$binary")"
find $firefoxoutdir/ -executable -type f | while read i; do
chmod u+w "$i"
[[ $i == *.so ]] || patchelf --set-interpreter "$interpreter" "$i"
patchelf --set-rpath "$rpath" "$i"
find $firefoxoutdir/ -executable -type f | while read file; do
chmod u+w "$file"
[[ $file == *.so ]] || patchelf --set-interpreter "$(patchelf --print-interpreter "$binary")" "$file"
patchelf --set-rpath "$(patchelf --print-rpath "$binary")" "$file"

chmod u-w "$i"

@SuperSandro2000 SuperSandro2000 Feb 17, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
chmod u-w "$i"

fixed by nix

done

# create the wrapper script
rm $firefoxoutdir/firefox
<"$wrapper" grep -vE '^exec ' > $firefoxoutdir/firefox

@SuperSandro2000 SuperSandro2000 Feb 17, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am not sure what this line does

echo "exec \"$firefoxoutdir/firefox-bin\" \"\$@\"" >> $firefoxoutdir/firefox
chmod a+x $firefoxoutdir/firefox
Comment on lines +183 to +184

@SuperSandro2000 SuperSandro2000 Feb 17, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We can't use wrapProgram?

'' + ''
FFMPEG_REVISION=$(jq -r '.browsers[] | select(.name == "ffmpeg").revision' $BROWSERS_JSON)
mkdir -p $out/ffmpeg-$FFMPEG_REVISION
Expand Down