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.

Sign up for GitHub

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

certbot: 0.9.3 -> 0.10.2 #22239

Closed
wants to merge 3 commits into from
Closed

certbot: 0.9.3 -> 0.10.2 #22239

wants to merge 3 commits into from

Conversation

pradeepchhetri
Copy link
Contributor

@pradeepchhetri pradeepchhetri commented Jan 28, 2017 •
edited

Motivation for this change
Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@mention-bot
Copy link

@pradeepchhetri, thanks for your PR! By analyzing the history of the files in this pull request, we identified @gebner, @FRidh and @domenkozar to be potential reviewers.

@kevincox
Copy link
Contributor

kevincox commented Jan 28, 2017 •
edited

This appears to break simp_le, could you please investigate?

@Mic92
Copy link
Member

Mic92 commented Jan 29, 2017 •
edited

See also #21174 regarding the last upgrade of simp_le.
@kevincox how does the package relate to simp_le? This pull request does not affect pythonPackages.acme.

@pradeepchhetri
Copy link
Contributor Author

pradeepchhetri commented Jan 29, 2017 •
edited

I tried debugging this: simp_le is failing with the following stacktrace: https://gist.github.com/pradeepchhetri/266b3d6e6fb471378f5b3c1a1bcd8a93

Looks like it is unable to get the version properly. I will debug this further and update the pull request accordingly. Thanks.

@kevincox
Copy link
Contributor

@Mic92 I can't say I know for certian but if you run nix-build -A simp_le on this commit if fails but it succeeds on the parent commit.

@pradeepchhetri
Copy link
Contributor Author

@kevincox @Mic92 I guess the issue is because simp_le expects acme version to be between 0.9 and 0.10 (https://github.com/zenhack/simp_le/blob/master/setup.py#L11) I have updated the pull request. Since I very new to nix, do you think this is the right approach. I would like to get your feedbacks. Thank you.

FRidh
FRidh reviewed Feb 10, 2017
View reviewed changes
pkgs/tools/admin/certbot/0.10.nix

postInstall = ''
for i in $out/bin/*; do
wrapProgram "$i" --prefix PYTHONPATH : "$PYTHONPATH" \
Copy link
Member

Choose a reason for hiding this comment

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

buildPythonApplication already creates wrappers that set PYTHONPATH in the fixupPhase, so there shouldn't be any need for including PYTHONPATH here.

Furthermore, never use --prefix PYTHONPATH because this can break in case you have a PYTHONPATH set in your environment.

@pradeepchhetri pradeepchhetri mentioned this pull request Feb 17, 2017
Closed
@fpletz
Copy link
Member

fpletz commented Feb 21, 2017

Certbot was updated in 72cda16.

@fpletz fpletz closed this Feb 21, 2017
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

Assignees
No one assigned
Labels
6.topic: python 8.has: package (update)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@pradeepchhetri @mention-bot @kevincox @Mic92 @fpletz @FRidh @pSub