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

kmscon: 8 -> unstable-2018-09-07 #94701

Merged
merged 2 commits into from Aug 10, 2020
Merged

kmscon: 8 -> unstable-2018-09-07 #94701

merged 2 commits into from Aug 10, 2020

Conversation

peterhoeg
Copy link
Member

Motivation for this change

Also move to a new upstream that is updated to work with recent packages
in nixpkgs.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • 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 nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@asdf8dfafjk
Copy link
Contributor

asdf8dfafjk commented Aug 7, 2020

I reviewed this PR.

If anybody else wants to review- here are the steps:

	disabledModules = [
		# "virtualisation/virtualbox-host.nix"
		"services/ttys/kmscon.nix"
	];
	imports =
	[
		<LOCALCHECKOUT>/nixos/modules/services/ttys/kmscon.nix"
        ]

       # IMPORT THIS PR AS A NIXPKG AND IN THE ABOVE LOCAL CHECKOUT, REPLACE THE PACAKGE

Review, source

  1. Hashes of both libtsm and kmscon match the implied source address
  2. libtsm no longer builds on darwin, although I'm not sure why it used to since rg'ing through nixpkgs tells me that kmscon was the sole user (removed patch and buildinputs)
  3. libtsm no longer has --disable-debug flag fro configure
  4. kmscon removes patch, I verified in the new source that the two lines are as in the patch, iterate until SIGSYS, and an extra #include.

This looks good, so long as @peterhoeg did (3) deliberately. Also, perhaps add to meta.description a variation of "Updated fork of KMSCon, a KMS/DRM based system console" to clarify that this is no longer an "official" work...? I'm not sure of the best practice here really, so feel free to ignore this point.

Review, usage:

I was able to use kmscon in my tty, and the following things worked:

  • (Beautiful, as always) non-latin fonts
  • scrollback
  • zoom

Good to go. I can test anything else if any regular user has suggestions.

@peterhoeg
Copy link
Member Author

This looks good, so long as @peterhoeg did (3) deliberately.

Yep. libtsm is now built with cmake that handles this for us (and besides configureFlags have no effect on cmake)

Also, perhaps add to meta.description a variation of "Updated fork of KMSCon, a KMS/DRM based system console" to clarify that this is no longer an "official" work...?

We change upstream every now and then which normally isn't reflected in the description as it doesn't really add much.

@peterhoeg
Copy link
Member Author

Thanks for the thorough review @asdf8dfafjk. Much appreciated!

@peterhoeg peterhoeg merged commit 02d8704 into master Aug 10, 2020
@peterhoeg peterhoeg deleted the u/kmscon branch August 10, 2020 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants