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

[WIP] deepin: updates, new packages and fixes #52403

Closed
wants to merge 36 commits into from
Closed

[WIP] deepin: updates, new packages and fixes #52403

wants to merge 36 commits into from

Conversation

romildo
Copy link
Contributor

@romildo romildo commented Dec 16, 2018 •
edited

Motivation for this change

Updates, new packages and fixes for the Deepin Desktop Environment.

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 nox --run "nox-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.

@romildo
Copy link
Contributor Author

romildo commented Dec 16, 2018

@GrahamcOfBorg build deepin.dbus-factory deepin.dde-api deepin.dde-calendar deepin.dde-daemon deepin.dde-dock deepin.dde-file-manager deepin.dde-network-utils deepin.dde-polkit-agent deepin.dde-qt-dbus-factory deepin.dde-session-ui deepin.deepin-anything deepin.deepin-desktop-base deepin.deepin-desktop-schemas deepin.deepin-gettext-tools deepin.deepin-gtk-theme deepin.deepin-icon-theme deepin.deepin-image-viewer deepin.deepin-menu deepin.deepin-metacity deepin.deepin-movie-reborn deepin.deepin-mutter deepin.deepin-shortcut-viewer deepin.deepin-sound-theme deepin.deepin-terminal deepin.deepin-turbo deepin.deepin-wallpapers deepin.deepin-wm deepin.dpa-ext-gnomekeyring deepin.dtkcore deepin.dtkwm deepin.dtkwidget deepin.go-dbus-factory deepin.go-dbus-generator deepin.go-gir-generator deepin.go-lib deepin.qt5dxcb-plugin deepin.qt5integration deepin.startdde

@romildo
Copy link
Contributor Author

romildo commented Dec 16, 2018

@GrahamcOfBorg build deepin.dbus-factory deepin.dde-api deepin.dde-calendar deepin.dde-daemon deepin.dde-dock deepin.dde-file-manager deepin.dde-network-utils deepin.dde-polkit-agent deepin.dde-qt-dbus-factory deepin.dde-session-ui deepin.deepin-anything deepin.deepin-desktop-base deepin.deepin-desktop-schemas deepin.deepin-gettext-tools deepin.deepin-gtk-theme deepin.deepin-icon-theme deepin.deepin-image-viewer deepin.deepin-menu deepin.deepin-metacity deepin.deepin-movie-reborn deepin.deepin-mutter deepin.deepin-shortcut-viewer deepin.deepin-sound-theme deepin.deepin-terminal deepin.deepin-turbo deepin.deepin-wallpapers deepin.deepin-wm deepin.dpa-ext-gnomekeyring deepin.dtkcore deepin.dtkwm deepin.dtkwidget deepin.go-dbus-factory deepin.go-dbus-generator deepin.go-gir-generator deepin.go-lib deepin.qt5dxcb-plugin deepin.qt5integration deepin.startdde

This was referenced Dec 18, 2018
Closed
Closed
worldofpeace
worldofpeace reviewed Dec 18, 2018
View reviewed changes
pkgs/desktops/deepin/setup-hook.sh
@@ -0,0 +1,33 @@
# Helper functions for deepin packaging
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should tell Deepin about a thing called PATH? 🤣
Stuff like hardcoding /bin/ls shouldn't be needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are plenty of binaries (and other files) that are hardcoded like that on DDE. It is going to give some work to have all of them fixed for NixOS.

Copy link
Contributor Author

@romildo romildo Dec 19, 2018 •
edited

Choose a reason for hiding this comment

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

There are at least two ways of fixing some of the hard coded paths for binaries:

  • fix its path (/usr/bin/dde-file-manager becomes ${dde-file-manager}/bin/dde-file-manager in deepin-screenshot, for instance); it may result in an additional run time dependency
  • remove the directory, leaving only the file name (/usr/bin/dde-file-manager becomes dde-file-manager); it will work only if the directory is in $PATH, and the dependency may have to be explicitly installed

Some days ago someone told me on IRC that the first approach is better. But I am not sure of that, because sometimes the dependency may be optional and costly. Nonetheless I have adopted it.

Any opinions?

@flokli
Copy link
Contributor

flokli commented Dec 19, 2018

@romildo can you cherry-pick flokli@813cb24 (add deepin.deepin-screenshot)?

Fixes #52358.

@worldofpeace
Copy link
Contributor

@romildo Btw, buildGoPackage and stdenv.mkDerviation both default name from pname and version.

Might be something you want to drop later.

@romildo
Copy link
Contributor Author

romildo commented Dec 19, 2018

Btw, buildGoPackage and stdenv.mkDerviation both default name from pname and version.

Might be something you want to drop later.

@worldofpeace Should just removing (or commenting) the name attribute work? I have done that and I am getting the message:

error: undefined variable 'name' at /alt/nixpkgs/pkgs/desktops/deepin/deepin-screenshot/default.nix:41:48

@romildo
Copy link
Contributor Author

romildo commented Dec 19, 2018

can you cherry-pick flokli/nixpkgs@813cb24 (add deepin.deepin-screenshot)?

Added, with some additional dependencies and hard coded paths fixed.

@worldofpeace
Copy link
Contributor

Btw, buildGoPackage and stdenv.mkDerviation both default name from pname and version.
Might be something you want to drop later.

@worldofpeace Should just removing (or commenting) the name attribute work? I have done that and I am getting the message:

error: undefined variable 'name' at /alt/nixpkgs/pkgs/desktops/deepin/deepin-screenshot/default.nix:41:48

It seems that you're expecting the name attribute with

passthru.updateScript = deepin.updateScript { inherit name; };

In that case I don't think it would be much added benefit to add that change, aside from conforming.

@romildo
Copy link
Contributor Author

romildo commented Dec 19, 2018 •
edited

I am trying to add new packages, updates and fixes for DDE in small portions, to make the PRs more manageable. Otherwise a lot of changes would pile up making them difficult to review.

At one point (I hope soon) a NixOS service to start the DDE from the login manager will also be added.

@romildo
Copy link
Contributor Author

romildo commented Dec 19, 2018

@GrahamcOfBorg build deepin.deepin-screenshot

@worldofpeace
Copy link
Contributor

I am trying to add new packages, updates and fixes for DDE in small portions, to make the PRs more manageable. Otherwise a lot of changes would pile up make them difficult to review.

I agree that can be a better approach that what I have done recently 😉

At one point (I hope soon) a NixOS service to start the DDE from the login manager will also be added.

Should be easier to implement now that we have 'upstream' session support. And they seem to be compliant with that https://github.com/linuxdeepin/startdde/blob/master/misc/xsessions/deepin.desktop.in

@romildo
Copy link
Contributor Author

romildo commented Dec 19, 2018

At one point (I hope soon) a NixOS service to start the DDE from the login manager will also be added.

Should be easier to implement now that we have 'upstream' session support.

Is there any documentation on how to make use of 'upstream' session on NixOS?

@worldofpeace
Copy link
Contributor

At one point (I hope soon) a NixOS service to start the DDE from the login manager will also be added.

Should be easier to implement now that we have 'upstream' session support.

Is there any documentation on how to make use of 'upstream' session on NixOS?

You should set this option to [ deepin.startdde ] in the deepin desktop-manager module.

Though I would start with getting a greeter module which should be simple because they're using lightdm.
(those go here)

@Swizz
Copy link

Swizz commented Jan 17, 2019

Once merged, will Deepin be available for NixOS as Desktop Manager and usable through the services.xserver.desktopManager.deepin.enable config line ?

@romildo
Copy link
Contributor Author

romildo commented Jan 19, 2019

Once merged, will Deepin be available for NixOS as Desktop Manager and usable through the services.xserver.desktopManager.deepin.enable config line ?

No yet. I am still working on making it work.

@romildo romildo changed the title deepin: updates, new packages and fixes [WIP] deepin: updates, new packages and fixes Jan 19, 2019
@romildo
Copy link
Contributor Author

romildo commented Mar 31, 2019

Closing in favor of #58634.

@romildo romildo closed this Mar 31, 2019
This was referenced Apr 3, 2019
Merged
Merged
@romildo romildo deleted the upd.deepin branch November 13, 2019 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@worldofpeace worldofpeace worldofpeace left review comments

@infinisil infinisil Awaiting requested review from infinisil

@flokli flokli Awaiting requested review from flokli

Assignees
No one assigned
Labels
2.status: work-in-progress 6.topic: nixos 8.has: documentation 8.has: module (update) 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 11-100 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@romildo @flokli @worldofpeace @Swizz @infinisil @GrahamcOfBorg