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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

certbot: 0.39.0 -> 1.0.0 #75663

Merged
merged 3 commits into from Dec 30, 2019
Merged

certbot: 0.39.0 -> 1.0.0 #75663

merged 3 commits into from Dec 30, 2019

Conversation

flokli
Copy link
Contributor

@flokli flokli commented Dec 14, 2019 •
edited

This bumps certbot from 0.39.0 to their latest release, 1.0.0.

I also got the unit tests to work again 馃帀. We don't need to pass in pebble, or run the certbot-ci tests, as elaborated in certbot/certbot#7450.

This is still pending on zenhack/simp_le#131, as simp_le currently doesn't want to build with acme-1.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 nix-review --run "nix-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.
Notify maintainers

cc @

@ofborg ofborg bot requested a review from domenkozar December 14, 2019 19:04
@flokli flokli marked this pull request as ready for review December 27, 2019 15:10
@flokli
Copy link
Contributor Author

flokli commented Dec 27, 2019

@GrahamcOfBorg test acme

@flokli
Copy link
Contributor Author

flokli commented Dec 27, 2019

simpl_le was bumped to a version compatible with our acme python module, and I got the acme vm test to past, marked as "Ready for review".

@ofborg ofborg bot requested review from makefu and gebner December 27, 2019 15:19
Br1ght0ne
Br1ght0ne reviewed Dec 27, 2019
View reviewed changes
Copy link
Member

@Br1ght0ne Br1ght0ne left a comment

Choose a reason for hiding this comment

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

Checked with nixpkgs-review, found out that hass-nabucasa can't build because of it depends on acme==0.39.0 (strongly).

[7 built (2 failed), 155 copied (201.4 MiB), 29.9 MiB DL]
error: build of '/nix/store/18cjv2hq4791vrvnwvhv06m0f1xplw2q-env.drv' failed
https://github.com/NixOS/nixpkgs/pull/75663
2 package failed to build:
python37Packages.hass-nabucasa python38Packages.hass-nabucasa

5 package built:
certbot python27Packages.acme python37Packages.acme python38Packages.acme simp_le

@flokli
Copy link
Contributor Author

flokli commented Dec 27, 2019 via email

@picnoir
Copy link
Member

picnoir commented Dec 28, 2019

Ping @Scriptkiddi, maintainer of Hass. Could you have a look? We really need to bump Certbot.

@makefu
Copy link
Contributor

makefu commented Dec 29, 2019

@NinjaTrappeur @flokli we could simply make the hass-nabucasa update part of this PR, also the question is if the pin to 0.39 is really necessary (it is not according to the PR linked).
So there are two choices:

  1. update hass-nabucasa to the latest dev release
  2. remove the acme version pin with sed in the patchPhase

i would prefer 2

makefu
makefu approved these changes Dec 29, 2019
View reviewed changes
Copy link
Contributor

@makefu makefu left a comment

Choose a reason for hiding this comment

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

haven't tested it but code looks good to me!

pkgs/tools/admin/certbot/default.nix
'';

dontUseSetuptoolsCheck = true;
doCheck = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

i love it when tests get re-enabled for packages 馃憤

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I'm also feeling much more confident with tests being enabled.

@flokli
Copy link
Contributor Author

flokli commented Dec 29, 2019 •
edited

@makefu relaxed the version bounds for hass-nabucasa, tests still pass. Also upstreamed to NabuCasa/hass-nabucasa#119 , PTAL.

@ofborg ofborg bot requested a review from makefu December 29, 2019 19:30
@Scriptkiddi
Copy link
Contributor

lgtm

@makefu
Copy link
Contributor

makefu commented Dec 29, 2019

@ofborg build python38Packages.hass-nabucasa

@makefu
Copy link
Contributor

makefu commented Dec 29, 2019

@ofborg build python37Packages.hass-nabucasa
tested the issue, seems like yarl does not build with python38 (but python37)

@makefu makefu mentioned this pull request Dec 29, 2019
Closed
@makefu
Copy link
Contributor

makefu commented Dec 29, 2019 •
edited

For the bump of yarl i used the following code in pkgs/development/python-modules/yarl/default.nix :

{ stdenv
, fetchPypi
, buildPythonPackage
, multidict
, pytest
, pytestcov
, idna
}:

buildPythonPackage rec {
  pname = "yarl";
  version = "1.4.2";

  src = fetchPypi {
    inherit pname version;
    sha256 = "0jzpgrdl6415zzl8js7095q8ks14555lhgxah76mimffkr39rkaq";
  };
  checkPhase = ''
    pytest ./tests ./yarl
  '';

  checkInputs = [ pytest pytestcov ];
  propagatedBuildInputs = [ multidict idna ];

  meta = with stdenv.lib; {
    description = "Yet another URL library";
    homepage = https://github.com/aio-libs/yarl/;
    license = licenses.asl20;
    maintainers = with maintainers; [ dotlambda ];
  };
}

@flokli
Copy link
Contributor Author

flokli commented Dec 30, 2019

Some of yarl's test being broken isn't really connected to this PR.

Let's track this upstream at aio-libs/yarl#410 and merge this PR, it;s already broken before.

@flokli flokli merged commit 7ef8a85 into NixOS:master Dec 30, 2019
@flokli flokli deleted the certbot-1.0.1 branch December 30, 2019 01:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FRidh FRidh FRidh left review comments

@Br1ght0ne Br1ght0ne Br1ght0ne left review comments

@fpletz fpletz Awaiting requested review from fpletz

@picnoir picnoir Awaiting requested review from picnoir

@domenkozar domenkozar Awaiting requested review from domenkozar

@gebner gebner Awaiting requested review from gebner

@jonringer jonringer Awaiting requested review from jonringer

@makefu makefu Awaiting requested review from makefu

Assignees
No one assigned
Labels
6.topic: python 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@flokli @picnoir @makefu @Scriptkiddi @FRidh @Br1ght0ne