udisks slimming #42324
Closed
udisks slimming #42324
+29
−12
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
GrahamcOfBorg
added
6.topic: nixos
|
Success on x86_64-darwin (full log) Attempted: pcsclite The following builds were skipped because they don't evaluate on x86_64-darwin: libselinux, libsepol, volume_key Partial log (click to expand)
|
|
Success on x86_64-linux (full log) Attempted: libselinux, libsepol, pcsclite, volume_key Partial log (click to expand)
|
|
Success on aarch64-linux (full log) Attempted: libselinux, libsepol, pcsclite, volume_key Partial log (click to expand)
|
orivej
reviewed
Jun 21, 2018
| # This URL changes in unpredictable ways, so it is not sensible | ||
| # to put a version variable in there. | ||
| url = "https://pcsclite.apdu.fr/files/pcsc-lite-1.8.23.tar.bz2"; | ||
| url = "https://pcsclite.apdu.fr/files/pcsc-lite-${version}.tar.bz2"; |
There was a problem hiding this comment.
This was done in 4d5565e. Could you rebase?
There was a problem hiding this comment.
Does it matter? It should still merge cleanly.
There was a problem hiding this comment.
Not much; it only makes history and blame view a bit confusing.
There was a problem hiding this comment.
I will rebase it before merging, do not have enough free space for another build.
orivej
reviewed
Jun 21, 2018
| ExecStart = "${pkgs.pcsclite}/sbin/pcscd -f -x -c ${cfgFile}"; | ||
| ExecReload = "${pkgs.pcsclite}/sbin/pcscd -H"; | ||
| ExecStart = "${stdenv.lib.getBin pkgs.pcsclite}/sbin/pcscd -f -x -c ${cfgFile}"; | ||
| ExecReload = "${stdenv.lib.getBin pkgs.pcsclite}/sbin/pcscd -H"; |
There was a problem hiding this comment.
Here and elsewhere lib is already available. Could you use lib instead of stdenv.lib?
There was a problem hiding this comment.
Will do. By the way, do you, by any chance, know why are packages encouraged to use stdenv.lib instead of lib but the converse is used for modules?
jtojnar
commented
Jun 21, 2018
| @@ -6,25 +6,31 @@ stdenv.mkDerivation rec { | |||
| se_release = "20170804"; | |||
| se_url = "https://raw.githubusercontent.com/wiki/SELinuxProject/selinux/files/releases"; | |||
|
|
|||
| outputs = [ "bin" "out" "dev" "man" ]; | |||
There was a problem hiding this comment.
It would be nice to split the static library (1.5M) out but I am not sure how to do it without breaking the pkgconfig use.
|
Success on x86_64-linux (full log) Attempted: libselinux, libsepol, pcsclite, volume_key Partial log (click to expand)
|
|
Success on aarch64-linux (full log) Attempted: libselinux, libsepol, pcsclite, volume_key Partial log (click to expand)
|
|
Success on x86_64-darwin (full log) Attempted: pcsclite The following builds were skipped because they don't evaluate on x86_64-darwin: libselinux, libsepol, volume_key Partial log (click to expand)
|
|
Even 297M sounds like a serious regression, since udisks' closure size in 18.03 was just 76 MiB. |
|
|
Success on x86_64-darwin (full log) Attempted: pcsclite The following builds were skipped because they don't evaluate on x86_64-darwin: libselinux, libsepol, udisksMinimal, volume_key Partial log (click to expand)
|
|
Failure on aarch64-linux (full log) Attempted: libselinux, libsepol, pcsclite, udisksMinimal, volume_key Partial log (click to expand)
|
|
Success on x86_64-linux (full log) Attempted: libselinux, libsepol, pcsclite, udisksMinimal, volume_key Partial log (click to expand)
|
|
It does not seem to be possible to remove systemd, devicemapper is required by cryptsetup which is required by libblockdev-crypto, which is mandatory dependency of udisks. Well, we could replace systemd with eudev in lvm2. It gets us from 198M totaldown to 163M total |
|
Changing 126MI do not think I can get it down anymore. @edolstra Using udev instead of eudev adds 47M the closure size but I guess it will already be in the closure, so it might not be that bad. 173M |
|
No attempt on x86_64-darwin (full log) The following builds were skipped because they don't evaluate on x86_64-darwin: libblockdev, libselinux, libsepol, lvm2, ntfs3g, parted, pcsclite, udisksMinimal, volume_key Partial log (click to expand)
|
|
Failure on x86_64-linux (full log) Attempted: libblockdev, libselinux, libsepol, lvm2, ntfs3g, parted, pcsclite, udisksMinimal, volume_key Partial log (click to expand)
|
|
Failure on aarch64-linux (full log) Attempted: libblockdev, libselinux, libsepol, lvm2, ntfs3g, parted, pcsclite, udisksMinimal, volume_key Partial log (click to expand)
|
|
We don't ATM; I understand that as a suggestion to reduce this closure. |
|
No attempt on x86_64-darwin (full log) The following builds were skipped because they don't evaluate on x86_64-darwin: libblockdev, libselinux, libsepol, lvm2, ntfs3g, parted, pcsclite, udisksMinimal, volume_key Partial log (click to expand)
|
|
Success on x86_64-linux (full log) Attempted: libblockdev, libselinux, libsepol, lvm2, ntfs3g, parted, pcsclite, udisksMinimal, volume_key Partial log (click to expand)
|
|
Oops, I didn't remember "resolve" to be among the magic words. |
|
Success on aarch64-linux (full log) Attempted: libblockdev, libselinux, libsepol, lvm2, ntfs3g, parted, pcsclite, udisksMinimal, volume_key Partial log (click to expand)
|
GrahamcOfBorg
added
the
2.status: merge conflict
infinisil
added
the
6.topic: closure size
It results in much smaller closure size.
Results in much smaller closure size.
GrahamcOfBorg
added
10.rebuild-darwin: 0
|
No attempt on x86_64-darwin (full log) The following builds were skipped because they don't evaluate on x86_64-darwin: lvm2, ntfs3g, udisksMinimal Partial log (click to expand)
|
|
Success on x86_64-linux (full log) Attempted: lvm2, ntfs3g, udisksMinimal Partial log (click to expand)
|
|
Failure on aarch64-linux (full log) Attempted: lvm2, ntfs3g, udisksMinimal Partial log (click to expand)
|
|
@jtojnar I tried bumping to that version, however, some of the Can you take a look? (Btw, while you're at it, we might also want to |
|
Haha okay, so you were monitoring that pagure PR as well :-) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation for this change
Removed
python2from the closure, reducing its size from 353M to 297M.Things done
sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)