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.

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

maxx: 1.1.0 -> 2.0.1 #59188

Merged
merged 1 commit into from Apr 18, 2019
Merged

maxx: 1.1.0 -> 2.0.1 #59188

merged 1 commit into from Apr 18, 2019

Conversation

ghost
Copy link

@ghost ghost commented Apr 8, 2019

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nix-review --run "nix-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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@GrahamcOfBorg GrahamcOfBorg added 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 1-10 labels Apr 8, 2019
@pSub pSub added the 2.status: work-in-progress This PR isn't done label Apr 9, 2019
@ghost ghost requested a review from infinisil as a code owner April 11, 2019 05:33
@GrahamcOfBorg GrahamcOfBorg added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Apr 11, 2019
@ghost
Copy link
Author

ghost commented Apr 11, 2019

I've dropped libredirect hacks in favor of creation of symlink from package in nix store to /opt/MaXX in maxx module like it was somewhere in the past. The reason is that icons of minimized programs on desktop are broken otherwise. I wasn't able to figure issue with strace and got no help in that regard from maxx author

@markuskowa markuskowa self-assigned this Apr 11, 2019
@ghost ghost changed the title [WIP] maxx: 1.1.0 -> 2.0.1 maxx: 1.1.0 -> 2.0.1 Apr 11, 2019
@markuskowa
Copy link
Member

It seems to work so far. Here are a couple of things:

  • hinv requires dmidecode, pciutils, and usbutils
  • The symbols in the iconbox look broken
    (might be my Virtualbox graphics)
  • Most entries in the tools chest menu do not work. For example "System Info", "Desktop Manager", "Search for Files"

@ghost
Copy link
Author

ghost commented Apr 14, 2019 •
edited by ghost
Loading

@markuskowa Thank you!

  1. fixed
  2. unfortunately don't know how to fix. I'll ask maxx author. As a workaround if we will not find a solution we could fallback to the previous release behaviour *useIconBox: False in Xdefaults. They will be shown on the desktop without issues like on this screen https://www.facebook.com/maxxdesktop/photos/rpp.1083070501713918/1557043887649908/?type=3&theater
  3. these functions are not finished, and so these items point to author specific environment tools
$ grep -e "System Info" -e "Desktop Manager" -e "Search For Files" result/opt/MaXX/etc/system.chestrc
        "System Info"           f.checkexec.sh.le "gnome-control-center info"
        "Desktop Manager"       f.checkexec.sh.le "gnome-control-center"
        "Search For Files"      f.checkexec.sh.le "gnome-search-tool"

These could be overriden in ~/.chestrc

markuskowa
markuskowa reviewed Apr 14, 2019
View reviewed changes
nixos/modules/services/x11/desktop-managers/maxx.nix
# there is hardcoded path in binaries
system.activationScripts.setup-maxx = ''
mkdir -p /opt
ln -sfn ${pkgs.maxx}/opt/MaXX /opt
Copy link
Member

Choose a reason for hiding this comment

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

That is not very pretty. Is there no other way to solve this?

Copy link
Author

Choose a reason for hiding this comment

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

@markuskowa Using libredirect, but that way minimized icons are totally broken. Also MaXX author strongly suggests using /opt/MaXX location.

Copy link
Member

Choose a reason for hiding this comment

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

It's unfortunate that the package is closed source.

@markuskowa
Copy link
Member

  1. unfortunately don't know how to fix. I'll ask maxx author. As a workaround if we will not find a solution we could fallback to the previous release behaviour *useIconBox: False in Xdefaults. They will be shown on the desktop without issues like on this screen https://www.facebook.com/maxxdesktop/photos/rpp.1083070501713918/1557043887649908/?type=3&theater

Setting the old behavior is probably the more clean version for now.

@markuskowa markuskowa merged commit dac0051 into NixOS:master Apr 18, 2019
@ghost
Copy link
Author

ghost commented Apr 19, 2019

@markuskowa Thank you!

@ghost ghost deleted the maxx branch April 19, 2019 05:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@infinisil infinisil Awaiting requested review from infinisil

Assignees

@markuskowa markuskowa

Labels
2.status: work-in-progress This PR isn't done 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@markuskowa @pSub @GrahamcOfBorg