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

Make the channel importer versions dynamic #455

Closed
wants to merge 23 commits into from

Conversation

dasJ
Copy link
Member

@dasJ dasJ commented Mar 29, 2022

I have no idea if this works…

Taken from: https://thekevinwang.com/2021/09/19/github-actions-dynamic-matrix/

@ysndr
Copy link
Member

ysndr commented Apr 2, 2022

Oh ive been looking for this for a while.
I thought you couldn't use expressions at the matrix side.

karantan added a commit to karantan/nixos-search that referenced this pull request Apr 7, 2022
This way it will be easier to update them because all the information
is in one JSON blob (inside Channels.elm file).

Ref: NixOS#455
karantan added a commit to karantan/nixos-search that referenced this pull request Apr 7, 2022
This way it will be easier to update them because all the information
is in one JSON blob (inside Channels.elm file).

Ref: NixOS#455
karantan added a commit to karantan/nixos-search that referenced this pull request Apr 7, 2022
This way it will be easier to update them because all the information
is in one JSON blob (inside Channels.elm file).

Ref: NixOS#455
karantan added a commit to karantan/nixos-search that referenced this pull request Apr 7, 2022
This way it will be easier to update them because all the information
is in one JSON blob (inside Channels.elm file).

Ref: NixOS#455
@garbas
Copy link
Member

garbas commented Apr 7, 2022

The channels are actually coming from this channels.nix file. I would prefer if we use that way of getting the channels.

Here would be the full plan:

  • 1. add nixos-org-configurations as input to flake.nix
  • 2. evaluate channels.nix file and export channels via environment variable. that environment variable (lets call it NIXOS_CHANNELS) should be present during the build and inside the nix shell. the content of the variable can be JSON.
  • 3. we pickup the NIXOS_CHANNELS environment variable in frontend/webpack.config.js and pass it further to webpack process, just like we do with ELASTICSEARCH_MAPPING_SCHEMA_VERSION.
  • 4. we forward NIXOS_CHANNELS to Elm via frontend/src/index.js as an Elm application flag. Just like we do with other variables there.
  • 5. we should then add a nixosChannels to Flags type alias in frontend/src/Main.elm and store it in the mode in the init function in the same file. We should also decode the JSON that is being passes in to ensure it is of correct shape.
  • 6. we should then change how we handle Channel in frontend/src/Search.elm to not type check anymore but to get all the information from the model.nixosChannels.
  • 7. also use NIXOS_CHANNELS variable with flake-info

Would somebody be willing to work on this? I can help out if you get stuck, either via matrix or short call.

/cc @dasJ @karantan

@garbas garbas self-requested a review April 7, 2022 15:23
1. add nixos-org-configurations as input to flake.nix
2. evaluate channels.nix file and export channels via environment variable.
   that environment variable (lets call it NIXOS_CHANNELS) should be present
   during the build and inside the nix shell. the content of the variable can
   be JSON.
3. we pickup the NIXOS_CHANNELS environment variable in
   frontend/webpack.config.js and pass it further to webpack process, just
   like we do with ELASTICSEARCH_MAPPING_SCHEMA_VERSION.
4. we forward NIXOS_CHANNELS to Elm via frontend/src/index.js as an Elm
   application flag. Just like we do with other variables there.
@garbas
Copy link
Member

garbas commented Apr 7, 2022

I went ahead and merge with current main and got first 4 items done.

@ncfavier
Copy link
Member

ncfavier commented Apr 8, 2022

I think this would mean we only get new channels when we update the flake inputs? Even if that process is semi-automated, I wonder if we could update the channels truly dynamically.

@garbas
Copy link
Member

garbas commented Apr 8, 2022

I think this would mean we only get new channels when we update the flake inputs? Even if that process is semi-automated, I wonder if we could update the channels truly dynamically.

I would like to have control over the channels. To help us reproduce the bugs. I don't think it is a problem if channel comes online a day later.

@ncfavier ncfavier self-requested a review April 8, 2022 09:29
flake.nix Outdated Show resolved Hide resolved
@karantan
Copy link
Contributor

karantan commented Apr 9, 2022

Would somebody be willing to work on this? I can help out if you get stuck, either via matrix or short call.

🤚 I think I can do it. Or at least give it a shot. I would also like to go on a short call if it's possible to make sure I'm going in the right direction.

@garbas
Copy link
Member

garbas commented Apr 10, 2022

Would somebody be willing to work on this? I can help out if you get stuck, either via matrix or short call.

raised_back_of_hand I think I can do it. Or at least give it a shot. I would also like to go on a short call if it's possible to make sure I'm going in the right direction.

Join us here and lets see when we both have time: https://matrix.to/#/#nixos-search:matrix.org

@ysndr
Copy link
Member

ysndr commented Apr 17, 2022

Pushed to the wrong repo, point 7 now in here too

@ysndr
Copy link
Member

ysndr commented Apr 17, 2022

I split the dev shells for frontend and flake info in the last commit since i dont think you should need to down load 4G of elm if you only work on the rust and vice verse.
Happy to make a new PR on top of this later if that is desired.

.github/workflows/import-to-elasticsearch.yml Outdated Show resolved Hide resolved
flake.nix Outdated Show resolved Hide resolved
flake.nix Outdated Show resolved Hide resolved
flake.nix Show resolved Hide resolved
flake.nix Outdated Show resolved Hide resolved
Comment on lines +424 to +426
/// Information about allowed and default nixos channels.
/// Typyically passed by environment variable NIXOS_CHANNELS.
/// Used to filter the input arguments for `flake-info nixpkgs` and `flake-info nixpkgs-archive`
Copy link
Member

Choose a reason for hiding this comment

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

Not sure I understand the point of checking this in flake-info. What if I want to run nixpkgs-archive locally on some random branch? Will that fail?

garbas and others added 5 commits April 20, 2022 20:54
Co-authored-by: Naïm Favier <n@monade.li>
but still provide devShells for each of the subprojects
and define lib = nixpkgs.lib before the call to eachDefaultSystem
Also, version = lib.fileContents ./VERSION;
Co-authored-by: Naïm Favier <n@monade.li>
@garbas
Copy link
Member

garbas commented Apr 20, 2022

Instead of writing a JSON file, how about instead exposing the nixos channels as a lib.nixosChannels output (defined outside of the eachDefaultSystems call), and in the workflow doing something like

I plan to reuse this JSON file also for other flakes. I guess we should rename it than to something else, but I guess we can do this at the later point in time, but this way we are prepared to grow the channels (or release branches) for other things then nixpkgs.

@garbas
Copy link
Member

garbas commented Apr 20, 2022

I split the dev shells for frontend and flake info in the last commit since i dont think you should need to down load 4G of elm if you only work on the rust and vice verse.
Happy to make a new PR on top of this later if that is desired.

I've reverted partially the change and made that both subprojects are there in devShell.default, but you can always select a subproject (nix develop .#flake-info) if you don't with to pull in the Elm stuff.

@@ -237,6 +237,9 @@ async fn run_command(
Ok((exports, ident))
}
Command::Nixpkgs { channel } => {
// TODO: if NIXOS_CHANNELS environment variable is present check
Copy link
Member

Choose a reason for hiding this comment

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

@karantan @ysndr This is where step number 7 should be implemented. I've also added the instructions

@garbas
Copy link
Member

garbas commented Apr 20, 2022

I'm closing this PR and open new one to provide a preview for changes.

@garbas garbas closed this Apr 20, 2022
@garbas garbas mentioned this pull request Apr 20, 2022
@github-actions github-actions bot temporarily deployed to pull request April 20, 2022 19:42 Inactive
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.

None yet

5 participants