Skip to content

Conversation

@trofi
Copy link
Contributor

@trofi trofi commented May 25, 2024

Changes:

Description of changes

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@trofi trofi marked this pull request as draft May 25, 2024 21:29
@trofi
Copy link
Contributor Author

trofi commented May 25, 2024

Needs a 7z test tweak: diffoscope> FAILED tests/comparators/test_sevenz.py::test_metadata_diff - AssertionError

@trofi
Copy link
Contributor Author

trofi commented May 25, 2024

diffoscope assumes that running 7z returns version in a second word:

def sevenz_version():
    try:
        out = subprocess.check_output(["7z"])
    except subprocess.CalledProcessError as e:
        out = e.output
    return out.decode("UTF-8").split()[1].strip()

But in nixpkgs we have an old 7z that does it in third word:

7z | head -n2

7-Zip [64] 17.05 : Copyright (c) 1999-2021 Igor Pavlov : 2017-08-28

@trofi
Copy link
Contributor Author

trofi commented May 25, 2024

@trofi trofi force-pushed the diffoscope-update branch from f466990 to 59ab4b9 Compare May 25, 2024 21:53
@trofi trofi marked this pull request as ready for review May 25, 2024 21:54
@trofi
Copy link
Contributor Author

trofi commented May 25, 2024

Ready for review.

@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels May 25, 2024
@trofi
Copy link
Contributor Author

trofi commented May 25, 2024

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

6 packages built:
  • diffoscope
  • diffoscope.dist
  • diffoscope.man
  • diffoscopeMinimal
  • diffoscopeMinimal.dist
  • diffoscopeMinimal.man

@trofi trofi force-pushed the diffoscope-update branch from 59ab4b9 to c5b49ba Compare May 26, 2024 09:34
@trofi
Copy link
Contributor Author

trofi commented May 26, 2024

Switched to commit hash merged to upstream master branch. No patch hash change.

@trofi
Copy link
Contributor Author

trofi commented May 26, 2024

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

6 packages built:
  • diffoscope
  • diffoscope.dist
  • diffoscope.man
  • diffoscopeMinimal
  • diffoscopeMinimal.dist
  • diffoscopeMinimal.man

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one person. label May 29, 2024
@trofi trofi force-pushed the diffoscope-update branch from c5b49ba to 553c41f Compare June 1, 2024 20:57
@trofi trofi changed the title diffoscope: 267 -> 268 diffoscope: 267 -> 269 Jun 1, 2024
@trofi
Copy link
Contributor Author

trofi commented Jun 1, 2024

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

6 packages built:
  • diffoscope
  • diffoscope.dist
  • diffoscope.man
  • diffoscopeMinimal
  • diffoscopeMinimal.dist
  • diffoscopeMinimal.man

@trofi
Copy link
Contributor Author

trofi commented Jun 1, 2024

Proposed the fix upstream as https://salsa.debian.org/reproducible-builds/diffoscope/-/merge_requests/142

269 release merged it. Updated to 269. Had to add a sourceRoot=./-269 workaround for a new https://salsa.debian.org/reproducible-builds/diffoscope/-/issues/378 issue.

@wegank wegank removed the 12.approvals: 1 This PR was reviewed and approved by one person. label Jun 2, 2024
@SuperSandro2000 SuperSandro2000 merged commit d0babee into NixOS:master Jun 2, 2024
@SuperSandro2000
Copy link
Member

good enough for now. That probably gets fixed in the next release

@trofi trofi deleted the diffoscope-update branch June 2, 2024 20:39
@trofi
Copy link
Contributor Author

trofi commented Jun 4, 2024

Had to add a sourceRoot=./-269 workaround for a new https://salsa.debian.org/reproducible-builds/diffoscope/-/issues/378 issue.

To make nixpkgs more resilient to directories with fancy root directory names proposed a few fixes as:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants