-
-
Notifications
You must be signed in to change notification settings - Fork 12.7k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
This will need more patching to work properly (especially for python builds), but I've been able to convince it to build some simple java and scala projects in its current form so I figured I'd spread it.
- Loading branch information
Showing
1 changed file
with
148 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
97bf063
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I very much feel like reverting this:
I can fully understand that someone doesn't have time to fix everything, we're all busy, but by not fixing the tests that load is shifted to someone else. Its very hard this way to tell whether a change (of a dependency) broke any of these packages.
97bf063
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
97bf063
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More accessible history and the possibility to cherry-pick individual packages.
97bf063
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
97bf063
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
97bf063
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If one chooses to cherry-pick the main dependent, pants (?), then yes. However, if one wants to cherry-pick one of the dependencies, then that's not true.
97bf063
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
97bf063
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
97bf063
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will you be annoyed if I add the meta tags in a single commit? 😄 I can break them up but it just feels like such busywork for questionable benefit...
97bf063
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, would it be weird to define a
twitter-commons
attrset inside pythonPackages rather than using all those hyphenated names?97bf063
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@FRidh when you say tests don't pass, what do you mean? I just tried building it under
build-use-sandbox
and the whole thing built fine. Do you have a particular thing that breaks from this commit?97bf063
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Build the Python 3 versions of these packages and observe.
97bf063
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@FRidh I still don't understand. It seems that by depending on fasteners (which I didn't add), I'm excluding python 3. Should I just disable it on python 3?
$ nix-build -A python3Packages.fasteners error: testtools-1.8.0 not supported for interpreter python3.5m
Sorry, I'm trying not to be dense here. Just trying to understand what's wrong with the above. I admit that I didn't test it on
python3Packages
and should have, but if it doesn't work because of a transitive dependency, I'm not sure what I can/should do about that.97bf063
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the case that you're quoting there is nothing to be done. I meant packages like
twitter-common-collections
which doesn't find any tests (in the case of Python 3).97bf063
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh!! I see, that makes sense. I'll fix those.
97bf063
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually couldn't get
twitter-common-collections
(or any of thetwitter-*
packages) to fail on python 3 (.4, .5, or .6) or 2. I did find an issue with python 3 onlmdb
and it wasn't clear what was going wrong so I just disabled checks on python3 for now. I'm now adding meta tags to the remaining packages. Anything I'm missing?97bf063
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@FRidh any thoughts on my question about making
twitter-common
a nested attrset insidepythonPackages
? Do other things assume we won't have nested attrsets?97bf063
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copumpkin yes, let's not do that. They're all just packages, and the names you've used are very similar to upstream names. Putting them in a set will complicate overriding the package set.
97bf063
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough. My main motivation was to factor out the common
meta
elements across them (license, site, maintainer) and a nested attrset would've given me a nice "local" place to put alet commonMeta = { };
😄97bf063
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@FRidh I think all the
meta
is dealt with now and they all seem to build fine on all python3* and python2. Thanks for all the help/feedback!