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

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

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 @

@flokli flokli force-pushed the flokli:certbot-1.0.1 branch from 7d842a3 to 1568b32 Dec 17, 2019
@flokli flokli force-pushed the flokli:certbot-1.0.1 branch from 1568b32 to 17a0a27 Dec 27, 2019
@flokli flokli marked this pull request as ready for review Dec 27, 2019
@flokli
Copy link
Contributor Author

@flokli flokli commented Dec 27, 2019

@GrahamcOfBorg test acme

@flokli
Copy link
Contributor Author

@flokli 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 Dec 27, 2019
Copy link
Contributor

@Br1ght0ne Br1ght0ne left a comment

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 flokli commented Dec 27, 2019

@NinjaTrappeur
Copy link
Member

@NinjaTrappeur NinjaTrappeur 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 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
Copy link
Contributor

@makefu makefu left a comment

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

'';

dontUseSetuptoolsCheck = true;
doCheck = true;

This comment has been minimized.

@makefu

makefu Dec 29, 2019
Contributor

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

This comment has been minimized.

@flokli

flokli Dec 29, 2019
Author Contributor

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

@flokli flokli force-pushed the flokli:certbot-1.0.1 branch from 17a0a27 to 8456f82 Dec 29, 2019
@flokli flokli requested a review from jonringer as a code owner Dec 29, 2019
@flokli
Copy link
Contributor Author

@flokli flokli commented Dec 29, 2019

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

@ofborg ofborg bot added the 6.topic: python label Dec 29, 2019
@ofborg ofborg bot requested a review from makefu Dec 29, 2019
@Scriptkiddi
Copy link
Contributor

@Scriptkiddi Scriptkiddi commented Dec 29, 2019

lgtm

@makefu
Copy link
Contributor

@makefu makefu commented Dec 29, 2019

@ofborg build python38Packages.hass-nabucasa

@makefu
Copy link
Contributor

@makefu makefu commented Dec 29, 2019

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

@makefu
Copy link
Contributor

@makefu makefu commented Dec 29, 2019

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 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
22 checks passed
22 checks passed
python38Packages.hass-nabucasa on aarch64-linux Failure
Details
python38Packages.hass-nabucasa on x86_64-darwin Failure
Details
python38Packages.hass-nabucasa on x86_64-linux Failure
Details
Evaluation Performance Report Evaluator Performance Report
Details
certbot on aarch64-linux Success
Details
certbot on x86_64-darwin Success
Details
certbot on x86_64-linux Success
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
python37Packages.hass-nabucasa on aarch64-linux Success
Details
python37Packages.hass-nabucasa on x86_64-darwin Success
Details
python37Packages.hass-nabucasa on x86_64-linux Success
Details
@flokli flokli deleted the flokli:certbot-1.0.1 branch Dec 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can鈥檛 perform that action at this time.