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

WIP: flexget: 2.10.40 -> 2.10.82 #28615

Closed
wants to merge 27 commits into from
Closed

WIP: flexget: 2.10.40 -> 2.10.82 #28615

wants to merge 27 commits into from

Conversation

Mic92
Copy link
Member

@Mic92 Mic92 commented Aug 27, 2017

Motivation for this change

needs more testing what dependent packages are broken by the update

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 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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@mention-bot
Copy link

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

@Mic92 Mic92 changed the title WIP: Flexget WIP: flexget: 2.10.40 -> 2.10.82 Aug 27, 2017
@Mic92 Mic92 force-pushed the flexget branch 2 times, most recently from dfaf880 to 8730f9d Compare August 27, 2017 16:50
@FRidh
Copy link
Member

FRidh commented Aug 27, 2017

There are a lot of Python package updates on staging, so let's wait till staging is merged into master.

@Mic92
Copy link
Member Author

Mic92 commented Aug 28, 2017

I will also merge this one into staging first.

@@ -15,15 +15,15 @@
}:

let
jinja = jinja2.override rec {
jinja = jinja2.overrideDerivation (old: rec {
Copy link
Member

Choose a reason for hiding this comment

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

check out the new overridePythonAttrs. It overrides the call to buildPythonPackage.

Copy link
Member Author

@Mic92 Mic92 Aug 28, 2017

Choose a reason for hiding this comment

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

Is this recommend inside python-modules? Also after all ansible should not be here after all.

Copy link
Member

Choose a reason for hiding this comment

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

Overriding within a derivation should indeed be prevented inside python-packages.nix to avoid multiple versions. But when you do override, I think overridePythonAttrs is typically most useful now.

I agree that ansible doesn't belong there, and if it would be moved the overriding wouldn't be an issue.

Copy link
Member Author

@Mic92 Mic92 Aug 28, 2017

Choose a reason for hiding this comment

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

I have move out ansible out of pythonPackages. In the documentation it is explained, how one can get a new python derivation with overridePythonAttrs, how does this work with buildPythonApplication?

Copy link
Member

Choose a reason for hiding this comment

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

Exactly the same.

@Mic92
Copy link
Member Author

Mic92 commented Sep 1, 2017

@tari if you spot some mistakes, please let me know.

@Mic92 Mic92 force-pushed the flexget branch 2 times, most recently from ae01232 to 0e491a5 Compare September 1, 2017 17:59
@FRidh
Copy link
Member

FRidh commented Sep 2, 2017

Thanks for also looking at this. A minor improvement would be to use checkInputs for the test dependencies so that if the tests are disabled, they won't actually be added to the build.

@Mic92
Copy link
Member Author

Mic92 commented Sep 2, 2017

Can you merge my changes into #28884
Or I would at least merge both at the same time into staging.

@FRidh
Copy link
Member

FRidh commented Sep 2, 2017

@Mic92 I've merged the changes in my branch so closing this.

@FRidh FRidh closed this Sep 2, 2017
@Mic92 Mic92 deleted the flexget branch September 2, 2017 11:49
@FRidh
Copy link
Member

FRidh commented Sep 5, 2017

I got

Collecting zxcvbn-python==4.4.15 (from FlexGet==2.10.82)
  Could not find a version that satisfies the requirement zxcvbn-python==4.4.15 (from FlexGet==2.10.82) (from versions: )
No matching distribution found for zxcvbn-python==4.4.15 (from FlexGet==2.10.82)
builder for ‘/nix/store/xns31jky1gw4vrdy56q2dgq59lwrhz5l-FlexGet-2.10.82.drv’ failed with exit code 1

with #28884.

@FRidh
Copy link
Member

FRidh commented Sep 5, 2017

Ahh, we need to unpin some dependencies. 5db60db

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.

None yet

4 participants