Skip to content

aws-azure-login: Link Puppeteer to Chromium#272592

Merged
imincik merged 2 commits intoNixOS:masterfrom
l0b0:fix-aws-azure-login-puppeteer-browser
Jan 15, 2024
Merged

aws-azure-login: Link Puppeteer to Chromium#272592
imincik merged 2 commits intoNixOS:masterfrom
l0b0:fix-aws-azure-login-puppeteer-browser

Conversation

@l0b0
Copy link
Contributor

@l0b0 l0b0 commented Dec 7, 2023

Description of changes

Closes #272544, avoiding the following runtime error:

Error: Could not find expected browser (chrome) locally. Run npm install to download the correct Chromium revision (982053).

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@l0b0 l0b0 requested review from dotlambda and yurrriq December 7, 2023 02:58
@l0b0 l0b0 force-pushed the fix-aws-azure-login-puppeteer-browser branch 3 times, most recently from 8621a75 to 7bbe9d9 Compare December 7, 2023 03:55
@ofborg ofborg bot added 8.has: clean-up This PR removes packages or removes other cruft 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Dec 7, 2023
@l0b0 l0b0 requested a review from dotlambda December 7, 2023 04:47
@l0b0 l0b0 force-pushed the fix-aws-azure-login-puppeteer-browser branch 2 times, most recently from 45e4f32 to a756bf8 Compare December 10, 2023 03:33
@ofborg ofborg bot added the 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. label Dec 10, 2023
@l0b0 l0b0 requested a review from yurrriq January 14, 2024 20:46
Copy link
Contributor

@imincik imincik left a comment

Choose a reason for hiding this comment

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

Thanks for fixing the package. Fix looks good to me but I'll made some comments and requested few changes. After that, I am good to merge this PR.

Here is the example of using finalAttrs function plus some other changes I requested:

diff --git a/pkgs/by-name/aw/aws-azure-login/package.nix b/pkgs/by-name/aw/aws-azure-login/package.nix
index e6eaa7fde372..164be566ee2f 100644
--- a/pkgs/by-name/aw/aws-azure-login/package.nix
+++ b/pkgs/by-name/aw/aws-azure-login/package.nix
@@ -1,28 +1,28 @@
 { lib
+, runCommand
 , stdenv
-, callPackage
+
 , chromium
 , fetchFromGitHub
 , fetchYarnDeps
 , makeWrapper
 , nodejs
-, pkgs
 , prefetch-yarn-deps
 , yarn
 }:
-stdenv.mkDerivation rec {
+stdenv.mkDerivation (finalAttrs: {
   pname = "aws-azure-login";
   version = "3.6.1";

   src = fetchFromGitHub {
     owner = "aws-azure-login";
     repo = "aws-azure-login";
-    rev = "v${version}";
+    rev = "v${finalAttrs.version}";
     hash = "sha256-PvPnqaKD98h3dCjEOwF+Uc86xCJzn2b9XNHHn13h/2Y=";
   };

   offlineCache = fetchYarnDeps {
-    yarnLock = "${src}/yarn.lock";
+    yarnLock = "${finalAttrs.src}/yarn.lock";
     hash = "sha256-SXQPRzF6b1FJl5HkyXNm3kGoNSDXux+0RYXBX93mOts=";
   };

@@ -69,7 +69,7 @@ stdenv.mkDerivation rec {
   '';

   passthru.tests.aws-azure-login =
-    pkgs.runCommand "${pname}-tests"
+    runCommand "${finalAttrs.pname}-tests"
       {
         HOME = "/tmp/home";
       } ''
@@ -84,7 +84,7 @@ stdenv.mkDerivation rec {
       azure_default_remember_me=false
       EOF

-      ! ${lib.getExe pkgs.aws-azure-login} --profile=my-profile 2> stderr
+      ! ${lib.getExe finalAttrs.finalPackage} --profile=my-profile 2> stderr
       [[ "$(cat stderr)" == 'Unable to recognize page state! A screenshot has been dumped to aws-azure-login-unrecognized-state.png. If this problem persists, try running with --mode=gui or --mode=debug' ]]

       touch $out
@@ -98,4 +98,4 @@ stdenv.mkDerivation rec {
     maintainers = with lib.maintainers; [ l0b0 ];
     platforms = lib.platforms.all;
   };
-}
+})

l0b0 added 2 commits January 15, 2024 22:03
The original maintainer no longer uses this software
<NixOS#272544 (comment)>.
Closes NixOS#272544.

The test verifies that the command fails for the *right* reason, rather
than the original issue:

> Error: Could not find expected browser (chrome) locally. Run `npm
> install` to download the correct Chromium revision (982053).
@l0b0 l0b0 force-pushed the fix-aws-azure-login-puppeteer-browser branch from a756bf8 to 44896b1 Compare January 15, 2024 09:16
@l0b0 l0b0 requested a review from imincik January 15, 2024 09:16
@imincik
Copy link
Contributor

imincik commented Jan 15, 2024

Result of nixpkgs-review pr 272592 run on x86_64-linux 1

1 package built:
  • aws-azure-login

Copy link
Contributor

@imincik imincik left a comment

Choose a reason for hiding this comment

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

Looks good to me now. Will merge once ofborg-eval is completed.

@github-actions
Copy link
Contributor

Successfully created backport PR for release-23.11:

@l0b0 l0b0 deleted the fix-aws-azure-login-puppeteer-browser branch January 15, 2024 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

8.has: clean-up This PR removes packages or removes other cruft 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

aws-azure-login can't find browser

4 participants