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

[staging] git: 2.44.1 -> 2.45.1 #312211

Merged
merged 2 commits into from
May 22, 2024
Merged

Conversation

JohnRTitor
Copy link
Contributor

@JohnRTitor JohnRTitor commented May 16, 2024

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.05 Release Notes (or backporting 23.05 and 23.11 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.

@JohnRTitor JohnRTitor marked this pull request as draft May 16, 2024 13:42
@JohnRTitor JohnRTitor changed the base branch from master to staging May 16, 2024 13:44
@JohnRTitor JohnRTitor marked this pull request as ready for review May 16, 2024 13:46
@JohnRTitor JohnRTitor requested a review from wmertens May 16, 2024 13:47
@JohnRTitor JohnRTitor changed the title git: 2.44.0 -> 2.45.1 git: 2.44.1 -> 2.45.1 May 16, 2024
@JohnRTitor JohnRTitor added the 1.severity: security Issues which raise a security issue, or PRs that fix one label May 16, 2024
@ofborg ofborg bot requested review from kashw2, primeos and globin May 16, 2024 14:42
@FlorianFranzen
Copy link
Contributor

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

3 packages built:
  • git
  • git.debug (git.debug.debug ,git.debug.doc)
  • git.doc (git.doc.debug ,git.doc.doc)

Copy link
Contributor

@Jayman2000 Jayman2000 left a comment

Choose a reason for hiding this comment

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

Notes for the “git: 2.44.1 -> 2.45.1” commit

I was able to build and run these packages on NixOS 23.11 (x86-64):

  • git
  • gitMinimal
  • gitSVN
  • gitFull

I tried cloning a repo with each of them, and they all worked. For this commit, I only have one small suggestion. You wrote the commit message like this:

git: 2.44.1 -> 2.45.1
Fixes  CVE-2024-32002, CVE-2024-32004, CVE-2024-32020,
CVE-2024-32021 and CVE-2024-32465
https://github.com/git/git/blob/v2.45.1/Documentation/RelNotes/2.45.1.txt

I would add a blank line in between the subject and the body of the commit:

git: 2.44.1 -> 2.45.1

Fixes  CVE-2024-32002, CVE-2024-32004, CVE-2024-32020,
CVE-2024-32021 and CVE-2024-32465
https://github.com/git/git/blob/v2.45.1/Documentation/RelNotes/2.45.1.txt

Notes for the “git: add passthru.update script” commit

The update script gives errors if git isn’t already up to date:

$ git switch update-git 
Switched to branch 'update-git'
Your branch is up to date with 'pr-JohnRTitor/update-git'.
$ git revert de05a8c1cf94dc7f5eee0074ca91cb0e1d2481cd
Auto-merging pkgs/applications/version-management/git/default.nix
[update-git c18a58b71366] Revert "git: 2.44.1 -> 2.45.1"
 1 file changed, 2 insertions(+), 2 deletions(-)
$ nix-shell maintainers/scripts/update.nix --argstr package git

Going to be running update for following packages:
 - git-2.44.1

Press Enter key to continue...

Running update for:
 - git-2.44.1: UPDATING ...
 - git-2.44.1: ERROR

--- SHOWING ERROR LOG FOR git-2.44.1 ----------------------

grep: warning: stray \ before {
warning: could not open directory 'pkgs/applications/version-management/git-and-tools/git/': No such file or directory
fatal: pathspec '/home/jayman/Documents/Home/VC/Git/Partially mine/nixpkgs/repo/pkgs/applications/version-management/git-and-tools/git/default.nix' did not match any files


--- SHOWING ERROR LOG FOR git-2.44.1 ----------------------
The update script for git-2.44.1 failed with exit code 128
$ 

Despite the errors, it does successfully update git.

@JohnRTitor JohnRTitor added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label May 17, 2024
@JohnRTitor JohnRTitor requested a review from Jayman2000 May 17, 2024 15:04
@JohnRTitor
Copy link
Contributor Author

I would add a blank line in between the subject and the body of the commit:

Done.

@JohnRTitor JohnRTitor requested a review from Aleksanaa May 17, 2024 15:06
@wegank wegank removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label May 17, 2024
Copy link
Contributor

@Jayman2000 Jayman2000 left a comment

Choose a reason for hiding this comment

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

I still get errors when I run the update script, but it does work despite those errors. Considering the fact that this PR fixes security issues, my recommendation is to merge it as is. Once this PR gets merged, I’ll open an issue for the update script errors.

Edit: It looks like someone beat me to it.

Copy link
Contributor

@SebTM SebTM left a comment

Choose a reason for hiding this comment

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

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

3 packages built:
  • git
  • git.debug (git.debug.debug ,git.debug.doc)
  • git.doc (git.doc.debug ,git.doc.doc)

@SebTM SebTM added the 12.approvals: 3+ This PR was reviewed and approved by three or more reputable people label May 17, 2024
@vcunat vcunat removed the 1.severity: security Issues which raise a security issue, or PRs that fix one label May 17, 2024
@vcunat
Copy link
Member

vcunat commented May 17, 2024

As the changelog says, this is not a security update. We got the fixes in PR #311801 already.

@JohnRTitor JohnRTitor changed the title git: 2.44.1 -> 2.45.1 [staging] git: 2.44.1 -> 2.45.1 May 18, 2024
@SuperSandro2000 SuperSandro2000 added 1.severity: security Issues which raise a security issue, or PRs that fix one and removed 1.severity: security Issues which raise a security issue, or PRs that fix one labels May 22, 2024
Comment on lines 320 to 334
+ (
if svnSupport then
''
# wrap git-svn
wrapProgram $out/libexec/git-core/git-svn \
--set GITPERLLIB "$out/${perlPackages.perl.libPrefix}:${
perlPackages.makePerlPath (perlLibs ++ [ svn.out ])
}" \
--prefix PATH : "${svn.out}/bin" ''
else
''
# replace git-svn by notification script
notSupported $out/libexec/git-core/git-svn
''
)
Copy link
Member

Choose a reason for hiding this comment

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

bruh, the formatter is just uglyfing the code

@SuperSandro2000
Copy link
Member

I've dropped the formatting commit because I wanted to merge this PR and it just felt wrong adding so much noise to the file without any urgency. Doing the formatting in one go with a proper entry to .git-blame-ignore-revs is just the way to go instead of gradually introducing it to all kinds of files and creating noise and maybe even hitting some early adopter bugs which would create some extra noise in the history.

@SuperSandro2000 SuperSandro2000 merged commit 31311bf into NixOS:staging May 22, 2024
6 of 7 checks passed
@JohnRTitor JohnRTitor deleted the update-git branch May 22, 2024 14:47
@me-and me-and mentioned this pull request May 24, 2024
13 tasks
@tuxiqae tuxiqae mentioned this pull request Jun 16, 2024
13 tasks
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.

7 participants