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

lxqt.libqtxdg: 3.3.1 -> 3.4.0 #72182

Merged
merged 2 commits into from Nov 2, 2019
Merged

lxqt.libqtxdg: 3.3.1 -> 3.4.0 #72182

merged 2 commits into from Nov 2, 2019

Conversation

romildo
Copy link
Contributor

@romildo romildo commented Oct 28, 2019

Motivation for this change

Update to version 3.4.0

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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@worldofpeace
Copy link
Contributor

This fails to build on darwin. Should any of these packages really have platforms.unix?

Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

nix-review passes on NixOS (1 broken on master)
diff LGTM
executables appear to work (not much testing)

[24 built (1 failed), 119 copied (568.8 MiB), 157.7 MiB DL]
error: build of '/nix/store/9q23f2pq6brgbpl041463kz1c940nmh1-env.drv' failed
https://github.com/NixOS/nixpkgs/pull/72182
1 package are marked as broken and were skipped:
deepin.deepin-wm

2 package failed to build:
deepin.dde-file-manager deepin.deepin-terminal

22 package were build:
deepin.dde-control-center deepin.deepin-image-viewer deepin.deepin-menu deepin.deepin-screenshot deepin.deepin-shortcut-viewer deepin.qt5integration lxqt.liblxqt lxqt.libqtxdg lxqt.lxqt-about lxqt.lxqt-admin lxqt.lxqt-config lxqt.lxqt-globalkeys lxqt.lxqt-notificationd lxqt.lxqt-openssh-askpass lxqt.lxqt-panel lxqt.lxqt-policykit lxqt.lxqt-powermanagement lxqt.lxqt-qtplugin lxqt.lxqt-runner lxqt.lxqt-session lxqt.lxqt-sudo lxqt.screengrab

I would switch the platform to linux though, it makes no mention of it supporting darwin.

@romildo
Copy link
Contributor Author

romildo commented Oct 29, 2019

This fails to build on darwin. Should any of these packages really have platforms.unix?

It seems that mariadb-connector-c is what fails on darwin, and as a consequence qtbase also fails, making this package also to fail. Is that enough to justify restricting it to the linux platform?

That would prevent having some applications (like screengrab) that depend on libqtxdg to be available in other platforms.

@romildo
Copy link
Contributor Author

romildo commented Oct 29, 2019

Should any of these packages really have platforms.unix?

That would imply changing most of the LXQt packages to platforms.unix, right?

@worldofpeace
Copy link
Contributor

@romildo No, that would be platforms.linux. Are these supported on darwin? I find it unlikely, it's a DE for linux so I assume linux is only supported.

libqtxdg fails to build on darwin.

LXQt is a DE for linux so assume darwin is not supported.

The platform for some packages that may be useful outside of
LXQt (like pcmanfm-qt, lximage-qt and others) and do not depend on
libqtxdg are not being changed.
@jonringer
Copy link
Contributor

@GrahamcOfBorg build lxqt.libqtxdg

@romildo
Copy link
Contributor Author

romildo commented Nov 2, 2019

Resticting the platform to linux, as it fails to build on darwin.

Doing the same for other components of LXQt that depends on libqtxdg.

Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

nix-review passes on NixOS (failures are broken on master)
diff LGTM
commits LGTM

[24 built (1 failed), 91 copied (551.4 MiB), 162.4 MiB DL]
error: build of '/nix/store/966qihw11zj9kpwng4z4cifcfrjv8lpd-env.drv' failed
https://github.com/NixOS/nixpkgs/pull/72182
2 package failed to build:
deepin.dde-file-manager deepin.deepin-terminal

23 package were build:
deepin.dde-control-center deepin.dde-kwin deepin.deepin-image-viewer deepin.deepin-menu deepin.deepin-screenshot deepin.deepin-shortcut-viewer deepin.qt5integration lxqt.liblxqt lxqt.libqtxdg lxqt.lxqt-about lxqt.lxqt-admin lxqt.lxqt-config lxqt.lxqt-globalkeys lxqt.lxqt-notificationd lxqt.lxqt-openssh-askpass lxqt.lxqt-panel lxqt.lxqt-policykit lxqt.lxqt-powermanagement lxqt.lxqt-qtplugin lxqt.lxqt-runner lxqt.lxqt-session lxqt.lxqt-sudo lxqt.screengrab

@jonringer jonringer merged commit a61bd1e into NixOS:master Nov 2, 2019
@romildo romildo deleted the upd.libqtxdg branch November 2, 2019 21:59
@worldofpeace
Copy link
Contributor

Resticting the platform to linux, as it fails to build on darwin.

Doing the same for other components of LXQt that depends on libqtxdg.

Thanks for fixing this 👍

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

3 participants