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

Jump to bottom

squashfs: use -no-hardlinks for reproducible squashfs images #114454

Merged
merged 1 commit into from Feb 28, 2021
Merged

squashfs: use -no-hardlinks for reproducible squashfs images #114454

merged 1 commit into from Feb 28, 2021

Conversation

raboof
Copy link
Member

@raboof raboof commented Feb 26, 2021

Motivation for this change

the nix store may contain hardlinks: derivations may output them
directly, or users may be using store optimization which automatically
hardlinks identical files in the nix store.

The presence of these links are intended to be a 'transparent'
optimization. However, when creating a squashfs image, the image
will be different depending on whether hard links were present
on the filesystem, leading to reproducibility problems.

By passing '-no-hardlinks' to mksquashfs the files are stored
as duplicates in the squashfs image. Since squashfs has support
for duplicate files this does not lead to a larger image.

For more details see
#114331

Things done

Tested by manually creating a squashfs image with jfsutils both with
hard links and with duplicate files. Verified this now leads to
identical squashfs images.

  • 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.

This was referenced Feb 26, 2021
Closed
Closed
@ofborg ofborg bot requested a review from ruuda February 26, 2021 10:39
@r-rmcgibbo
Copy link

r-rmcgibbo commented Feb 26, 2021 •
edited

Result of nixpkgs-review pr 114454 at 9b8bfab run on aarch64-linux 1

1 package marked as broken and skipped:
  • spotifywm
12 packages built:

Result of nixpkgs-review pr 114454 at 9b8bfab run on x86_64-linux 1

18 packages failed to build:
38 packages built:

primeos
primeos approved these changes Feb 26, 2021
View reviewed changes
Copy link
Member

@primeos primeos left a comment

Choose a reason for hiding this comment

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

LGTM :)

@SuperSandro2000
Copy link
Member

This is a semi-automatic executed nixpkgs-review with nixpkgs-review-checks extension. It is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch).
If you have any questions or problems please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 114454 run on x86_64-linux 1

1 package blacklisted:
  • appimage-run-tests
56 packages built:
  • Sylk
  • _1password-gui
  • appimage-run
  • appimagekit
  • apple-music-electron
  • authy
  • chrysalis
  • deltachat-electron
  • devdocs-desktop
  • diffoscope
  • distrobuilder
  • electronplayer
  • irccloud
  • jitsi-meet-electron
  • joplin-desktop
  • keeweb
  • ledger-live-desktop
  • lens
  • lunar-client
  • lxd
  • marktext
  • minetime
  • molotov
  • mycrypto
  • notable
  • nuclear
  • p3x-onenote
  • pcloud
  • plexamp
  • python37Packages.binwalk
  • python37Packages.binwalk-full
  • python38Packages.binwalk
  • python38Packages.binwalk-full
  • python39Packages.binwalk
  • python39Packages.binwalk-full
  • radicle-upstream
  • rambox-pro
  • ripcord
  • runwayml
  • singularity
  • soulseekqt
  • spotify
  • spotify-unwrapped
  • spotifywm
  • squashfsTools
  • ssb-patchwork
  • standardnotes
  • station
  • timeular
  • todoist-electron
  • trezor-suite
  • tusk
  • unityhub
  • wootility
  • zettlr
  • zulip

The following issues got detected with the above build packages.
Please fix at least the ones listed with your changed packages:

_1password-gui:

warning: missing-phase-hooks
installPhase should probably contain runHook preInstall and runHook postInstall.

Near pkgs/applications/misc/1password-gui/default.nix:29:3:

   |
29 |   installPhase = let
   |   ^

See: https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/missing-phase-hooks.md

appimagekit:

warning: maintainers-missing
Package does not have a maintainer. Consider adding yourself?

Near pkgs/tools/package-management/appimagekit/default.nix:112:3:

    |
112 |   meta = with lib; {
    |   ^

See: https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/maintainers-missing.md
warning: missing-patch-comment
Please add a comment on the line above, explaining the purpose of this patch.
Near pkgs/tools/package-management/appimagekit/default.nix:31:7:

   |
31 |       "${appimagekit_src}/squashfuse.patch"
   |       ^

See: https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/missing-patch-comment.md
warning: missing-patch-comment
Please add a comment on the line above, explaining the purpose of this patch.
Near pkgs/tools/package-management/appimagekit/default.nix:32:7:

   |
32 |       "${appimagekit_src}/squashfuse_dlopen.patch"
   |       ^

See: https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/missing-patch-comment.md
warning: missing-patch-comment
Please add a comment on the line above, explaining the purpose of this patch.
Near pkgs/tools/package-management/appimagekit/default.nix:65:15:

   |
65 |   patches = [ ./nix.patch ];
   |               ^

See: https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/missing-patch-comment.md

appimage-run:

warning: unused-argument
Unused argument: paths.
Near pkgs/build-support/trivial-builders.nix:284:12:

    |
284 |          , paths
    |            ^
appimage-run-tests:

warning: unused-argument
Unused argument: paths.
Near pkgs/build-support/trivial-builders.nix:284:12:

    |
284 |          , paths
    |            ^
diffoscope:

warning: missing-patch-comment
Please add a comment on the line above, explaining the purpose of this patch.
Near pkgs/tools/misc/diffoscope/default.nix:29:5:

   |
29 |     ./ignore_links.patch
   |     ^

See: https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/missing-patch-comment.md

ledger-live-desktop:

warning: unused-argument
Unused argument: makeDesktopItem.
Near pkgs/applications/blockchains/ledger-live-desktop/default.nix:1:18:

  |
1 | { lib, fetchurl, makeDesktopItem, appimageTools, imagemagick }:
  |                  ^
lxd:

error: no-flags-spaces
buildFlags cannot contain spaces, please use buildFlagsArray in Bash.

Near pkgs/tools/admin/lxd/default.nix:43:3:

   |
43 |   buildFlags = [ "-tags libsqlite3" ];
   |   ^

See: https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/no-flags-spaces.md

pcloud:

warning: missing-phase-hooks
installPhase should probably contain runHook preInstall and runHook postInstall.

Near pkgs/applications/networking/pcloud/default.nix:65:3:

   |
65 |   installPhase = ''
   |   ^

See: https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/missing-phase-hooks.md

python37Packages.binwalk-full:

warning: license-missing
Package is missing a license

Near pkgs/development/python-modules/binwalk/default.nix:53:3:

   |
53 |   meta = with lib; {
   |   ^

See: https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/license-missing.md

python37Packages.binwalk:

warning: license-missing
Package is missing a license

Near pkgs/development/python-modules/binwalk/default.nix:53:3:

   |
53 |   meta = with lib; {
   |   ^

See: https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/license-missing.md

python38Packages.binwalk-full:

warning: license-missing
Package is missing a license

Near pkgs/development/python-modules/binwalk/default.nix:53:3:

   |
53 |   meta = with lib; {
   |   ^

See: https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/license-missing.md

python38Packages.binwalk:

warning: license-missing
Package is missing a license

Near pkgs/development/python-modules/binwalk/default.nix:53:3:

   |
53 |   meta = with lib; {
   |   ^

See: https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/license-missing.md

python39Packages.binwalk-full:

warning: license-missing
Package is missing a license

Near pkgs/development/python-modules/binwalk/default.nix:53:3:

   |
53 |   meta = with lib; {
   |   ^

See: https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/license-missing.md

python39Packages.binwalk:

warning: license-missing
Package is missing a license

Near pkgs/development/python-modules/binwalk/default.nix:53:3:

   |
53 |   meta = with lib; {
   |   ^

See: https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/license-missing.md

ripcord:

warning: unused-argument
Unused argument: makeWrapper.
Near pkgs/applications/networking/instant-messengers/ripcord/default.nix:3:54:

  |
3 |   autoPatchelfHook, desktop-file-utils, imagemagick, makeWrapper,
  |                                                      ^

warning: unused-argument
Unused argument: zlib.
Near pkgs/applications/networking/instant-messengers/ripcord/default.nix:4:56:

  |
4 |   twemoji-color-font, xorg, libsodium, libopus, libGL, zlib, alsaLib }:
  |                                                        ^
singularity:

warning: unused-argument
Unused argument: go.
Near pkgs/applications/virtualization/singularity/default.nix:8:3:

  |
8 | , go
  |   ^
soulseekqt:

warning: maintainers-missing
Package does not have a maintainer. Consider adding yourself?

Near pkgs/applications/networking/p2p/soulseekqt/default.nix:56:5:

   |
56 |     maintainers = [ ];
   |     ^

See: https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/maintainers-missing.md
warning: missing-phase-hooks
installPhase should probably contain runHook preInstall and runHook postInstall.

Near pkgs/applications/networking/p2p/soulseekqt/default.nix:30:3:

   |
30 |   installPhase = ''
   |   ^

See: https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/missing-phase-hooks.md

spotifywm:

warning: missing-phase-hooks
installPhase should probably contain runHook preInstall and runHook postInstall.

Near pkgs/applications/audio/spotifywm/default.nix:17:3:

   |
17 |   installPhase = ''
   |   ^

See: https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/missing-phase-hooks.md

standardnotes:

warning: unused-argument
Unused argument: runtimeShell.
Near pkgs/applications/editors/standardnotes/default.nix:2:13:

  |
2 | , fetchurl, runtimeShell, libsecret, gtk3, gsettings-desktop-schemas }:
  |             ^
timeular:

warning: no-uri-literals
URI literals are deprecated.
Near pkgs/applications/office/timeular/default.nix:40:16:

   |
40 |     homepage = https://timeular.com;
   |                ^

See: https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/no-uri-literals.md

@raboof
squashfs: use -no-hardlinks for reproducible squashfs images
1521c83 
the nix store may contain hardlinks: derivations may output them
directly, or users may be using store optimization which automatically
hardlinks identical files in the nix store.

The presence of these links are intended to be a 'transparent'
optimization. However, when creating a squashfs image, the image
will be different depending on whether hard links were present
on the filesystem, leading to reproducibility problems.

By passing '-no-hardlinks' to mksquashfs the files are stored
as duplicates in the squashfs image. Since squashfs has support
for duplicate files this does not lead to a larger image.

For more details see
NixOS#114331
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/what-are-your-goals-for-21-05/11559/4

zimbatm
zimbatm approved these changes Feb 28, 2021
View reviewed changes
Copy link
Member

@zimbatm zimbatm left a comment

Choose a reason for hiding this comment

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

nice!

ruuda
ruuda approved these changes Feb 28, 2021
View reviewed changes
Copy link
Contributor

@ruuda ruuda left a comment

Choose a reason for hiding this comment

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

Nice, LGTM!

@zimbatm zimbatm merged commit 0aeba64 into NixOS:master Feb 28, 2021
15 checks passed
@raboof raboof mentioned this pull request Mar 4, 2021
Closed
@raboof raboof mentioned this pull request Jun 2, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SuperSandro2000 SuperSandro2000 SuperSandro2000 left review comments

@zimbatm zimbatm zimbatm approved these changes

@ruuda ruuda ruuda approved these changes

@primeos primeos primeos approved these changes

Assignees
No one assigned
Labels
6.topic: nixos 6.topic: reproducible builds 10.rebuild-darwin: 1-10 10.rebuild-linux: 11-100
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@raboof @r-rmcgibbo @SuperSandro2000 @nixos-discourse @zimbatm @ruuda @primeos