Skip to content

i3: add configFile to enable cutom configuration locations #26983

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
Merged
merged 1 commit into from
Jul 1, 2017
Merged

i3: add configFile to enable cutom configuration locations #26983

merged 1 commit into from
Jul 1, 2017
+69 −54

Conversation

Ma27
Copy link
Member

@Ma27 Ma27 commented Jun 30, 2017

Motivation for this change

i3 loads its configuration from ~/.config/i3, but in nix-based systems
you might want to build the config in ~/.nix-profile using a nix
derivation, so i3 needs to know where to look for the configuration
file.

I confirmed the change running nix-build -A i3 with a i3.configFile parameter in config.nix to ensure that i3 builds properly and I created a test VM using the following configuration:

{
  test = { pkgs, lib, ... }: with lib; {
    users.extraUsers.vm = {
      isNormalUser = true;
      password = "vm";
      extraGroups = [ "wheel" ];
    };

    nixpkgs.config = {
      i3.configFile = "/home/vm/i3.conf";
    };

    services.xserver = {
      enable = true;
      windowManager = {
        i3.enable = true;
        default = "i3";
      };
      desktopManager = {
        xterm.enable = false;
        default = "none";
      };
    };
  };
}
Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

Sorry, something went wrong.

@bennofs
Copy link
Contributor

bennofs commented Jun 30, 2017 •
edited
Loading

This forces a rebuild of i3 every time you change the config, which feels suboptimal. A better way would be to have an i3WithConfig, so you could do:

  i3 = i3WithConfig ./path/to/i3/config

i3WIthConfig would be separate from i3 and would only do the wrapping.

@Ma27
Copy link
Member Author

Ma27 commented Jun 30, 2017

good point, thanks for the comment.
I'll hope that I'll find some time tonight to refactor my change :-)

@bennofs bennofs self-assigned this Jun 30, 2017
@bennofs bennofs added the 2.status: work-in-progress This PR isn't done label Jun 30, 2017
@Ma27
Copy link
Member Author

Ma27 commented Jun 30, 2017

@bennofs I refactored my change (inspired by the termite derivation).
In the end the code uses the i3 derivation itself or a symlinkJoin construct based on the i3 derivation.
This avoids a complete rebuild of i3 in case of a config location change. On the other hand it keeps both components in one expression to avoid that the windowManagers module needs extra-logic to distinguish from i3 and some i3-config package

@Ma27
i3: add configFile to enable cutom configuration locations
dd4c1e2 
i3 loads its configuration from `~/.config/i3`, but in nix-based systems
you might want to build the config in `~/.nix-profile` using a nix
derivation, so `i3` needs to know where to look for the configuration
file.
@bennofs
Copy link
Contributor

bennofs commented Jul 1, 2017

Looks good, thank you!

@bennofs bennofs merged commit be4fc9e into NixOS:master Jul 1, 2017
@Ma27 Ma27 deleted the i3/allow-custom-configuration branch July 2, 2017 07:17
@grahamc
Copy link
Member

grahamc commented Jul 2, 2017 •
edited
Loading

FWIW this was unneeded:

  services = {
    xserver = {
      windowManager.i3 = {
        enable = true;
        configFile = ./i3.config;
      };

IMO we should revert this change to avoid duplicate / confusing configuration changes

Note, searching for i3 in the options shows this: https://nixos.org/nixos/options.html#i3

@grahamc grahamc mentioned this pull request Jul 2, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@bennofs bennofs

Labels
2.status: work-in-progress This PR isn't done
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Ma27 @bennofs @grahamc