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

tahoe-lafs: 1.13.0 -> unstable-2019-04-08 #59258

Closed
wants to merge 1 commit into from

Conversation

@manveru
Copy link
Contributor

@manveru manveru commented Apr 10, 2019

Motivation for this change

The new version has a lot of improvements and is required to get Gridsync to work.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Copy link
Member

@dotlambda dotlambda left a comment

Please make a separate commit for each package.

# Still a lot of tests are failing, need some help to debug this.
# checkPhase = ''
# trial --rterrors allmydata
# '';

This comment has been minimized.

@dotlambda

dotlambda Apr 10, 2019
Member

Please keep the checkPhase and set doCheck = false.
We might be able to help if you post the output of a test run.

owner = "tahoe-lafs";
repo = "tahoe-lafs";
rev = "325c522d7c73065e3f7e10518a9785b822d1405a";
sha256 = "0000000000000000000000000000000000000000000000000000";

This comment has been minimized.


pythonPackages.buildPythonApplication rec {
version = "1.13.0";
version = "unstable-2019-04-08";
name = "tahoe-lafs-${version}";
namePrefix = "";

This comment has been minimized.

@dotlambda

dotlambda Apr 10, 2019
Member

Suggested change
namePrefix = "";
@@ -18,7 +18,7 @@ buildPythonPackage rec {
'';

# Needs some fixing for 3.7
doCheck = !isPy37;
doCheck = !isPy3k;

This comment has been minimized.

@dotlambda

dotlambda Apr 10, 2019
Member

Why did you change this?

This comment has been minimized.

@manveru

manveru Apr 10, 2019
Author Contributor

It didn't pass tests on python 3.8, and isPy38 doesn't exist.

# rm eliot/tests/test_prettyprint.py
# '';

buildInputs = [ ];

This comment has been minimized.

@dotlambda

dotlambda Apr 10, 2019
Member

Suggested change
buildInputs = [ ];
, boltons
, hypothesis
, testtools
, pytest

This comment has been minimized.

@dotlambda

dotlambda Apr 10, 2019
Member

Most of these aren't used.

sha256 = "1fsw8rsn1s3nklx06mayrg5rn2zbky6wwjc5z07s7rf1wjzfs1wn";
};

checkPhase = "true"; # tests require internet

This comment has been minimized.

@dotlambda

dotlambda Apr 10, 2019
Member

Suggested change
checkPhase = "true"; # tests require internet
# tests require internet
doCheck = false;
@@ -0,0 +1,24 @@
{ stdenv, buildPythonPackage, fetchPypi

This comment has been minimized.

@dotlambda

dotlambda Apr 10, 2019
Member

Suggested change
{ stdenv, buildPythonPackage, fetchPypi
{ lib, buildPythonPackage, fetchPypi
rm eliot/tests/test_prettyprint.py
'';

buildInputs = [ zope_interface hypothesis ];

This comment has been minimized.

@dotlambda

dotlambda Apr 10, 2019
Member

Why are these buildInputs?

@@ -0,0 +1,36 @@
{ stdenv, buildPythonPackage, fetchPypi

This comment has been minimized.

@dotlambda

dotlambda Apr 10, 2019
Member

Suggested change
{ stdenv, buildPythonPackage, fetchPypi
{ lib, buildPythonPackage, fetchPypi
@dotlambda
Copy link
Member

@dotlambda dotlambda commented Apr 10, 2019

@manveru Did you ask upstream to make a release or do you know when to expect one?

@manveru
Copy link
Contributor Author

@manveru manveru commented Apr 10, 2019

@dotlambda so, this was mostly for @MostAwesomeDude to take a look at, not a final PR. Still thanks for all the feedback, will try to address it when I get a bit more time.

I asked them about a new release when I reported excessive memory and CPU usage. They don't know yet when it'll happen, so I thought we can just as well get this updated for now.

Maybe we should make a separate package for it?

@dotlambda
Copy link
Member

@dotlambda dotlambda commented Apr 10, 2019

this was mostly for @MostAwesomeDude to take a look at, not a final PR

This should be made obvious from the PR title/description.

Maybe we should make a separate package for it?

We can decide this once the PR is ready.

@manveru
Copy link
Contributor Author

@manveru manveru commented Apr 10, 2019

@dotlambda not going to complain, I'm actually happy this got reviewed, usually my PRs sit around for a few weeks unless I ping someone specifically ;)

@manveru manveru force-pushed the manveru:tahoe-lafs-update branch from 5cf9947 to def4f04 Apr 10, 2019
@exarkun exarkun mentioned this pull request Apr 11, 2019
2 of 10 tasks complete
@exarkun
Copy link
Contributor

@exarkun exarkun commented Apr 26, 2019

Not sure what test failures you saw, but looking at results I get locally, some of the test failures are due to some test files not being installed. This is fixable in Tahoe-LAFS itself with this patch:

diff --git a/setup.py b/setup.py
index 361e2c5dc..2e7cabfb4 100644
--- a/setup.py
+++ b/setup.py
@@ -249,7 +249,7 @@ setup(name="tahoe-lafs", # also set in __init__.py
                 "test": PleaseUseTox,
                 },
       package_dir = {'':'src'},
-      packages=find_packages('src'),
+      packages=find_packages('src') + ['allmydata.test.plugins'],
       classifiers=trove_classifiers,
       python_requires="<3.0",
       install_requires=install_requires,

@dotlambda
Copy link
Member

@dotlambda dotlambda commented Apr 26, 2019

some of the test failures are due to some test files not being installed

we don't want to install those as they are just for testing

@FRidh
Copy link
Member

@FRidh FRidh commented Feb 9, 2020

Nothing happening so closing.

@FRidh FRidh closed this Feb 9, 2020
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

5 participants
You can’t perform that action at this time.