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

biliass: 1.3.5 -> 1.3.4 #184012

Merged
merged 1 commit into from
Sep 4, 2022
Merged

biliass: 1.3.5 -> 1.3.4 #184012

merged 1 commit into from
Sep 4, 2022

Conversation

linsui
Copy link
Contributor

@linsui linsui commented Jul 30, 2022

Description of changes

No idea why biliass suddenly stop building due to dep mismatch.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/1050

Comment on lines 6990 to 6996
protobuf4_21 = callPackage ../development/python-modules/protobuf {
disabled = isPyPy;
# If a protobuf upgrade causes many Python packages to fail, please pin it here to the previous version.
doCheck = !isPy3k;
protobuf = pkgs.protobuf3_21;
};

Copy link
Member

Choose a reason for hiding this comment

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

This is not allowed in pythoNPackages. Please do this locally in the package with python package set override.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@@ -2,20 +2,20 @@
, buildPythonPackage
, fetchPypi
, pythonOlder
, protobuf
, protobuf4_21
Copy link
Member

Choose a reason for hiding this comment

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

This is not allowed in pythonPackages because it will cause issues if another protobuf is in the same python env.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What's the proper fix?

Copy link
Member

Choose a reason for hiding this comment

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

Move this to an python package override outside of pythonPackages

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean that I should add it to all-packages.nix or add it as a local override as I do?

Copy link
Member

Choose a reason for hiding this comment

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

It should be used as a local overwrite in the package that requires it which must be outside of python-modules

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I should move biliass out of python-modules?

Copy link
Member

Choose a reason for hiding this comment

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

Ideally if it would be only a CLI which it isn't. Sigh. I am not sure what the best approach is here.

Copy link
Member

Choose a reason for hiding this comment

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

We cannot handle multiple versions of a package in the same package set. Thus, if you really want it available as an importable library, it simply needs to reuse the protobuf used there. If that's not possible, then we just need to stick with the older version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We cannot handle multiple versions of a package in the same package set.

Is this a bug? What's a package set?

Thus, if you really want it available as an importable library, it simply needs to reuse the protobuf used there.

What's an importable library?

If that's not possible, then we just need to stick with the older version.

Do you mean patch the source code to downgrade the lib? By the way, any idea why it suddenly failed to build?

Copy link
Member

Choose a reason for hiding this comment

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

Is this a bug?

No, a limitation of python.

What's a package set?

eg python39Packages or python310Packages

By the way, any idea why it suddenly failed to build?

It probably failed to build because of an upgrade that happened somewhere.

@linsui linsui changed the title yutto: 2.0.0b13 -> 2.0.0b14 biliass: 1.3.5 -> 1.3.4 Aug 18, 2022
@linsui
Copy link
Contributor Author

linsui commented Aug 18, 2022

Looks like it's easier to downgrade biliass for now.

@linsui
Copy link
Contributor Author

linsui commented Aug 29, 2022

@SuperSandro2000 Can this be merged?

@SuperSandro2000 SuperSandro2000 merged commit 2ee7ea0 into NixOS:master Sep 4, 2022
@linsui linsui deleted the biliass branch September 4, 2022 00:58
@linsui
Copy link
Contributor Author

linsui commented Sep 4, 2022

Thanks!

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