pythonPackages.feedparser: 5.2.1 -> 6.0.1 #101635

delroth · 2020-10-25T08:50:22Z

Motivation for this change

Feedparser 6.0.1 was released on Sept 15. The 6.x branch is not compatible with Python 2.x anymore.

I found two packages in nixpkgs that do not work with 6.x at this point, with no upstream fix for either of them. To minimize breakage, I've kept feedparser 5.2.1 (current version) in nixpkgs as pythonPackages.feedparser5. I don't feel strongly about this (anything that doesn't have a plan to support Py3k in 2020 is probably not actively maintained anyway...), let me know if you'd rather me remove the relevant commits and break the reverse dependencies.

I've also fixed the derivation to add a checkPhase.

Things done

SuperSandro2000 · 2020-10-25T13:28:18Z

Result of nixpkgs-review pr 101635 1

1 package marked as broken and skipped:

kmymoney

6 packages failed to build:

tangogps (foxtrotgps)
quodlibet
quodlibet-full
quodlibet-without-gst-plugins
quodlibet-xine
quodlibet-xine-full

21 packages built:

appdaemon
calibre
canto-curses
canto-daemon
flexget
gpodder
greg
pubs
python27Packages.feedparser5
python37Packages.feedparser
python37Packages.feedparser5
python37Packages.sgmllib3k
python37Packages.weboob
python38Packages.feedparser
python38Packages.feedparser5
python38Packages.sgmllib3k
python38Packages.weboob
sabnzbd
tartube
tribler
ytcc

Fork of the existing feedparser package, branched since 6.x breaks Python 2.x compatiblity.

)

Ideally we would use tox, but upstream doesn't seem to include tox.ini in their sdist at the moment.

SuperSandro2000 · 2020-10-25T13:32:31Z

I think they fails are unrelated to this PR:

wrapping `/nix/store/hzs5pfk5r3s3b4khrvcj83j7bmvirxmi-quodlibet-xine-full-4.2.1/bin/.operon-wrapped'...
Executing pythonRemoveTestsDir
Finished executing pythonRemoveTestsDir
@nix { "action": "setPhase", "phase": "installCheckPhase" }
running install tests
dbus[666]: Failed to start message bus: Failed to open "/etc/dbus-1/session.conf": No such file or directory
/build/quodlibet-4.2.1/quodlibet/formats/_audio.py:1019: SyntaxWarning: "is" with a literal. Did you mean "=="?
  if profile is "none":
Gtk.init failed
/nix/store/3wsc5pi02dsbzbzyhrnm4a69xis42hn4-xvfb-run/bin/.xvfb-run-wrapped: line 186: kill: (657) - No such process

wrapping `/nix/store/7sf6fnkrkw4carhlaw2kf33q9n69ypis-quodlibet-full-4.2.1/bin/.operon-wrapped'...
Executing pythonRemoveTestsDir
Finished executing pythonRemoveTestsDir
@nix { "action": "setPhase", "phase": "installCheckPhase" }
running install tests
dbus[666]: Failed to start message bus: Failed to open "/etc/dbus-1/session.conf": No such file or directory
/build/quodlibet-4.2.1/quodlibet/formats/_audio.py:1019: SyntaxWarning: "is" with a literal. Did you mean "=="?
  if profile is "none":
Gtk.init failed
/nix/store/3wsc5pi02dsbzbzyhrnm4a69xis42hn4-xvfb-run/bin/.xvfb-run-wrapped: line 186: kill: (657) - No such process

delroth · 2020-10-25T13:33:34Z

Note that the set of quodlibet packages is already broken at HEAD for an unrelated reason according to Hydra. tangogps builds fine locally, do you have a repro? (/nix/store/xldsw4rzp0scp19vbni6lw4bx5jwg68g-foxtrotgps-1.2.2)

marsam · 2020-11-04T10:08:23Z

pkgs/top-level/python-packages.nix

  feedparser = callPackage ../development/python-modules/feedparser { };
+  feedparser5 = callPackage ../development/python-modules/feedparser/5.x.nix { };  # For Python 2.x compatibility.


Suggested change

feedparser = callPackage ../development/python-modules/feedparser { };

feedparser5 = callPackage ../development/python-modules/feedparser/5.x.nix { }; # For Python 2.x compatibility.

feedparser = if isPy3k then

callPackage ../development/python-modules/feedparser { };

else

callPackage ../development/python-modules/feedparser/5.x.nix { };

I looked into doing that, but there are other reasons to split off feedparser5 separately. For example, octoprint right now is Py3k compatible but their setup.py requires feedparser<6.

We could likely patch octoprint in place, I suspect that this <6 requirement is just there for safety against incompatible version bumps. I don't feel strongly either way. WDYT?

I would like to avoid having many version of the same package

Pinning within python-modules is highly discouraged. This will potentially introduce incompatible versions in a user's environment as either the new pinned version or the original version will be imported at runtime, breaking one of the packages.

The preference to handling this is to relax version bounds in the "install_requires" field. (could be in setup.py, pyproject.toml, requirements or others). In most cases, packages are still compatible with small API changes which may warrant a major version bump. We use test suites to verify that the package still works correctly.

If the package is still incompatible with the latest major version, then the most proper way to handle this is make an issue with the upstream package to adopt the latest major version. Or if upstream is not very responsive, you are free patch the source to make it compatible.

In very few circumstances, two versions of the same package are allowed to exist if the packages are extremely difficult to package. Some examples of this are tensorflow, which has huge ecosystems built around it and is hard to package. Another is django, which has 2 actively developed versions, and large ecosystems built around each.

One exception to this is applications, due to the way buildPythonApplication and toPythonApplication functions work, the related derivations will not bleed dependencies between packages. If the package doesn't need to be imported by other python modules, then this package would be a good candidate to convert into application. You can look at https://github.com/NixOS/nixpkgs/blob/master/pkgs/tools/admin/awscli/default.nix as an example of using an overlay within a python application.

Info on buildPythonApplication can be found here.
Info on toPythonApplication can be found here.

SuperSandro2000 · 2020-11-17T09:06:33Z

pkgs/development/python-modules/feedparser/5.x.nix

+  };
+
+}


Suggested change

};

}

};

}

jonringer · 2020-11-18T06:26:37Z

pkgs/development/python-modules/sgmllib3k/default.nix

+    sha256 = "1s8jm3dgqabgf8x96931scji679qkhvczlv3qld4qxpsicfgns3q";
+  };
+
+  doCheck = false;  # No tests.


If tests are not available, then please use pythonImportsCheck to import the most important modules. This isn't as good as unit tests, but will usually give a good indication of run-time errors. Please see pythonImportsCheck documentation for more information.

jonringer · 2020-11-18T06:27:42Z

pkgs/top-level/python-packages.nix

@@ -2032,6 +2032,7 @@ in {
  feedgenerator = callPackage ../development/python-modules/feedgenerator { inherit (pkgs) glibcLocales; };

  feedparser = callPackage ../development/python-modules/feedparser { };
+  feedparser5 = callPackage ../development/python-modules/feedparser/5.x.nix { };  # For Python 2.x compatibility.


Suggested change

feedparser5 = callPackage ../development/python-modules/feedparser/5.x.nix { }; # For Python 2.x compatibility.

feedparser_5 = callPackage ../development/python-modules/feedparser/5.x.nix { }; # For Python 2.x compatibility.

jonringer · 2020-11-18T06:28:21Z

pkgs/top-level/python-packages.nix

  feedparser = callPackage ../development/python-modules/feedparser { };
+  feedparser5 = callPackage ../development/python-modules/feedparser/5.x.nix { };  # For Python 2.x compatibility.


I would like to avoid having many version of the same package

Pinning within python-modules is highly discouraged. This will potentially introduce incompatible versions in a user's environment as either the new pinned version or the original version will be imported at runtime, breaking one of the packages.

The preference to handling this is to relax version bounds in the "install_requires" field. (could be in setup.py, pyproject.toml, requirements or others). In most cases, packages are still compatible with small API changes which may warrant a major version bump. We use test suites to verify that the package still works correctly.

If the package is still incompatible with the latest major version, then the most proper way to handle this is make an issue with the upstream package to adopt the latest major version. Or if upstream is not very responsive, you are free patch the source to make it compatible.

In very few circumstances, two versions of the same package are allowed to exist if the packages are extremely difficult to package. Some examples of this are tensorflow, which has huge ecosystems built around it and is hard to package. Another is django, which has 2 actively developed versions, and large ecosystems built around each.

One exception to this is applications, due to the way buildPythonApplication and toPythonApplication functions work, the related derivations will not bleed dependencies between packages. If the package doesn't need to be imported by other python modules, then this package would be a good candidate to convert into application. You can look at https://github.com/NixOS/nixpkgs/blob/master/pkgs/tools/admin/awscli/default.nix as an example of using an overlay within a python application.

Info on buildPythonApplication can be found here.
Info on toPythonApplication can be found here.

jonringer · 2020-11-18T06:28:57Z

pkgs/development/python-modules/feedparser/default.nix

-  doCheck = false;
+  propagatedBuildInputs = [ sgmllib3k ];
+
+  checkPhase = "python tests/runtests.py";


Suggested change

checkPhase = "python tests/runtests.py";

checkPhase = "${python.interpreter} tests/runtests.py";

support alternate interpreters

jonringer · 2020-11-18T06:29:31Z

pkgs/development/python-modules/feedparser/5.x.nix

+  };
+
+  # lots of networking failures
+  doCheck = false;


Suggested change

doCheck = false;

doCheck = false;

If tests are not available, then please use pythonImportsCheck to import the most important modules. This isn't as good as unit tests, but will usually give a good indication of run-time errors. Please see pythonImportsCheck documentation for more information.

delroth · 2020-11-19T02:22:22Z

Sorry, this is ending up being much more effort than I'm willing to put into a package version bump that I'm not even officially maintaining. I'll keep in mind some of the comments here for future Python derivations I might end up writing, but I don't think putting the burden on me to fix many instances of existing technical debt is fair when I'm just trying to get nixpkgs more up to date.

Closing this PR to avoid sitting on it forever and discouraging other folks from trying to own that problem themselves.

jonringer · 2020-11-19T03:43:45Z

@delroth it's help mitigate users having broken packages at runtime. Sure, it's burdensome, and probably should have been included when initially added. But it's better to break during the build, than break at runtime.

Also, when doing mass package updates, it's either myself or @FRidh trying to stabilize the entire package set, E.g. #101636 . So, it's still important to have packages which demonstrate breakages during build.

delroth requested review from FRidh and jonringer as code owners October 25, 2020 08:50

ofborg bot added 6.topic: python 8.has: clean-up 8.has: package (new) labels Oct 25, 2020

ofborg bot requested a review from domenkozar October 25, 2020 09:02

ofborg bot added 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 labels Oct 25, 2020

delroth force-pushed the feedparser-6.0.1 branch from da541ad to 1649c8d Compare October 25, 2020 10:12

SuperSandro2000 mentioned this pull request Oct 25, 2020

pythonPackages.django-oauth-toolkit,django_lts,paperless: Fix evaluation #101643

Closed

10 tasks

delroth added 7 commits October 25, 2020 14:32

python3Packages.sgmllib3k: init at 1.0.0

1f4449a

pythonPackages.feedparser5: init at 5.2.1

084bee5

Fork of the existing feedparser package, branched since 6.x breaks Python 2.x compatiblity.

rawdog: switch to feedparser5 for py2 compatibility

058703d

rss2email: switch to feedparser5, broken with 6.0.1 (wking/rss2email#123

541555a

)

octoprint: pin to feedparser5, setup.py requires feedparser<6

e8c06da

python3Packages.feedparser: 5.2.1 -> 6.0.1

aa7714d

python3Packages.feedparser: run tests at build time

9aea33f

Ideally we would use tox, but upstream doesn't seem to include tox.ini in their sdist at the moment.

delroth force-pushed the feedparser-6.0.1 branch from 1649c8d to 9aea33f Compare October 25, 2020 13:34

marsam reviewed Nov 4, 2020

View reviewed changes

SuperSandro2000 reviewed Nov 17, 2020

View reviewed changes

pkgs/development/python-modules/feedparser/5.x.nix

Comment on lines +23 to +25

};

}

Copy link

Member

SuperSandro2000 Nov 17, 2020

Choose a reason for hiding this comment

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

Suggested change

};

}

};

}

jonringer reviewed Nov 18, 2020

View reviewed changes

delroth closed this Nov 19, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pythonPackages.feedparser: 5.2.1 -> 6.0.1 #101635

pythonPackages.feedparser: 5.2.1 -> 6.0.1 #101635

delroth commented Oct 25, 2020

SuperSandro2000 commented Oct 25, 2020

SuperSandro2000 commented Oct 25, 2020

delroth commented Oct 25, 2020

marsam Nov 4, 2020

delroth Nov 17, 2020

jonringer Nov 18, 2020

SuperSandro2000 Nov 17, 2020

jonringer Nov 18, 2020

jonringer Nov 18, 2020

jonringer Nov 18, 2020

jonringer Nov 18, 2020

jonringer Nov 18, 2020

delroth commented Nov 19, 2020

jonringer commented Nov 19, 2020

		feedparser = callPackage ../development/python-modules/feedparser { };
		feedparser5 = callPackage ../development/python-modules/feedparser/5.x.nix { }; # For Python 2.x compatibility.

-  };
-}
+  };
+}

	feedparser5 = callPackage ../development/python-modules/feedparser/5.x.nix { }; # For Python 2.x compatibility.
	feedparser_5 = callPackage ../development/python-modules/feedparser/5.x.nix { }; # For Python 2.x compatibility.

	checkPhase = "python tests/runtests.py";
	checkPhase = "${python.interpreter} tests/runtests.py";

pythonPackages.feedparser: 5.2.1 -> 6.0.1 #101635

pythonPackages.feedparser: 5.2.1 -> 6.0.1 #101635

Conversation

delroth commented Oct 25, 2020

Motivation for this change

Things done

SuperSandro2000 commented Oct 25, 2020

SuperSandro2000 commented Oct 25, 2020

delroth commented Oct 25, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

delroth commented Nov 19, 2020

jonringer commented Nov 19, 2020