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.

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

Add nix flake #75

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

kczulko
Copy link

@kczulko kczulko commented Jan 12, 2023

Hello,

I was having some problems with aws-google-auth (constantly returning Invalid username or password) and one of its github issues pointed me to this project. Since aws-google-auth got nixified some time ago and gsts wasn't, I decided to write some nix expr for gsts (I'm a nixos user) and publish it to the upstream. Not sure how good is it but it works.

Notes:

  1. playwright installation script wants to download browser (?) so I had to disable this step via env variable. More details here.
  2. User has to explicitly put browser path while running gsts (see the devShell setup - chromium was added there and its path is accessible via env variable).

Best regards,
Karol

@ruimarinho
Copy link
Owner

@kczulko why wouldn't the browser download component work automatically?

@kczulko
Copy link
Author

kczulko commented Mar 8, 2023

@ruimarinho
I think that's because the buildPhase in nix is pure, in the meaning that downloading something at this stage is considered as a jailbrake. If this would be allowed than the build would become unpredictable (e.g. no clear way howto calculate artifact hash etc.).

@ruimarinho
Copy link
Owner

Is there an easy to test this via https://github.com/LnL7/nix-docker or similar on macOS?

@kczulko
Copy link
Author

kczulko commented Mar 13, 2023

Is there an easy to test this via https://github.com/LnL7/nix-docker or similar on macOS?

Basically, in such case I was using cachix/install-nix-action like e.g. here. However, this slows down the build a little bit. For this case, I would suggest moving this to separate workflow (e.g. not required to be "green" for ongoing PRs), or just leave it without testing. The latter, in case of failures would get fixed by someone from community who wants to use this installation/build recipe.

@kczulko kczulko force-pushed the kczulko/add-nix-flake branch from 78a3a3e to dd98404 Compare September 20, 2023 07:33
@dephiros
Copy link

@ruimarinho @kczulko Just run into this recently while trying to install gsts on nixos. Any chance we can get this merged? 🙏

@kczulko
Copy link
Author

kczulko commented Sep 23, 2024

@dephiros
I am not using gsts now and it's not my top priority. You can either use my fork/branch or you may try to contribute to gsts mainline. There was no response from the maintainer for my last suggestion, which makes me think that adding gh workflow for checking this flake against arch matrix would be sufficient to merge this. I think so 🤔

@ruimarinho
Copy link
Owner

I can merge this as is but without guarantees it will work in the future due to limited testing.

@dephiros
Copy link

@ruimarinho , seems like the node version is a bit old. I can help test it tomorrow with a newer version before we merge?

@ruimarinho
Copy link
Owner

Yep, that works for me! Thanks for the help.

@dephiros
Copy link

This does not go as I expected and definitely a bit outside of my wheel house:

  • Testing the flake as is seems to use node_16 which is removed so it does not work anymore
  • I tried to use buildNpmPackage(similar to nixpkgs) but haven't gotten that approach working yet.

As an alternative, I file a ticket with nixpkgs to add gsts. . This would also allow gsts to be installed with nix

@Deliganli
Copy link

I am very new to nodejs stack, apologies if sounds naive, is it feasable to upgrade the minimum nodejs version the project needs?
That would enable the nix packaging and would also benefit the project with updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants