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

croc: 6.4.10 -> 8.0.2 #80607

Closed
wants to merge 1 commit into from
Closed

croc: 6.4.10 -> 8.0.2 #80607

wants to merge 1 commit into from

Conversation

equirosa
Copy link
Contributor

@equirosa equirosa commented Feb 20, 2020 •
edited

Motivation for this change

New version released: https://github.com/schollz/croc/releases/tag/v8.0.2
This version fixes some security issues dealing with unintentionally unencrypted communication between client and relay. (https://github.com/schollz/croc/releases/tag/v8.0.0)

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

@ofborg ofborg bot requested a review from kalbasit February 20, 2020 07:02
@HugoReeves
Copy link
Member

HugoReeves commented Feb 22, 2020 •
edited

  • Builds on MacOS

Issue: The version of croc reported by output does not match 6.4.11. See...

-> nix-build -A croc .
/nix/store/ips47s5xbj0cw1dwziff7n4mxyjs670d-croc-6.4.11
-> ./result/bin/croc --version
croc version v6.4.10-073569f

@equirosa Please check if the same occurs on Linux.
Avoid merging until this is resolved.

@equirosa
Copy link
Contributor Author

Please check if the same occurs on Linux.

Odd, it does. In fact, the version it reports seems to be the exact same as the previous one. But, it exposes features present only in v6.4.11. 🤔

@HugoReeves
Copy link
Member

@equirosa I'm going to investigate upstream, building croc from source via the 6.4.11 tag, to see what's happening.

@HugoReeves
Copy link
Member

@equirosa Ok so this is quite strange. If you checkout v6.4.11 via cli, and navigate to src/cli/cli.go the value for version is Version = "v6.4.10-073569f". However, if you find the tag via github releases, and browse code at the commit provided, see cli/cli.go at v6.4.11, the value is Version = "v6.4.11-bc0841d".
If I checkout master and run git log I get the following.

commit 4c56ec283d0b327a2cd843d0524ccb97ff598580
Author: Zack Scholl <zack.scholl@gmail.com>
Date:   Tue Feb 18 11:28:36 2020 -0800

    bump 6.4.11

commit bc0841d8a13f9ab3bf622ff9e2f86e59e1996f71 (tag: v6.4.11)
Author: Zack Scholl <zack.scholl@gmail.com>
Date:   Tue Feb 18 11:25:02 2020 -0800

    update deps

This just seems like a weird git thing. Because everything builds successfully on both operating systems, and we have done some investigation, I say we just merge as is. This shouldn't have a negative effect on others and will probably be fixed in the next release.

HugoReeves
HugoReeves approved these changes Feb 23, 2020
View reviewed changes
Copy link
Member

@HugoReeves HugoReeves left a comment

Choose a reason for hiding this comment

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

LGTM

@equirosa equirosa changed the title croc: 6.4.10 -> 6.4.11 croc: 6.4.10 -> 8.0.1 Mar 2, 2020
@ofborg ofborg bot requested a review from HugoReeves March 2, 2020 19:06
HugoReeves
HugoReeves approved these changes Mar 2, 2020
View reviewed changes
Copy link
Member

@HugoReeves HugoReeves left a comment •
edited

Choose a reason for hiding this comment

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

  • Builds on Macos

LGTM

@Moredread
Copy link
Contributor

I think it would be helpful to mention that it fixes security issues and link either to the release notes at https://github.com/schollz/croc/releases/tag/v8.0.0 and/or include the release text.

A backport to 20.03 and 19.09 might be good aswell.

@equirosa
Copy link
Contributor Author

equirosa commented Mar 3, 2020

I think it would be helpful to mention that it fixes security issues and link either to the release notes at https://github.com/schollz/croc/releases/tag/v8.0.0 and/or include the release text.

You're right, added additional info.

@equirosa equirosa changed the title croc: 6.4.10 -> 8.0.1 croc: 6.4.10 -> 8.0.2 Mar 6, 2020
@equirosa
Copy link
Contributor Author

equirosa commented Mar 6, 2020

Issue: The version of croc reported by output does not match 6.4.11. See...

-> nix-build -A croc .
/nix/store/ips47s5xbj0cw1dwziff7n4mxyjs670d-croc-6.4.11
-> ./result/bin/croc --version
croc version v6.4.10-073569f

Version 8.0.2 kinda addresses this: output of option -v now reports
croc version v8.0.1-2d6206b

additionally it has some better error messages

@ofborg ofborg bot requested a review from HugoReeves March 6, 2020 19:30
@equirosa
Copy link
Contributor Author

Closing as a newer version was merged in #82532

@equirosa equirosa closed this Mar 14, 2020
@equirosa equirosa deleted the croc branch March 14, 2020 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HugoReeves HugoReeves HugoReeves approved these changes

@kalbasit kalbasit Awaiting requested review from kalbasit

Assignees
No one assigned
Labels
10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@equirosa @HugoReeves @Moredread