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

Update EDA tools #79572

Merged
merged 12 commits into from Feb 9, 2020
Merged

Update EDA tools #79572

merged 12 commits into from Feb 9, 2020

Conversation

@emilazy
Copy link
Member

@emilazy emilazy commented Feb 8, 2020

Motivation for this change

A big batch of updates to the free FPGA tooling, including new package python3Packages.nmigen-soc.

This should also fix trellis and its integration for ECP5 support in nextpnr, which wasn't previously working due to CMake installation path issues.

cc @thoughtpolice if you don't mind taking a look at this? :)

@GrahamcOfBorg build symbiyosys python3Packages.nmigen-boards python3Packages.nmigen-soc glasgow tinyprog

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Copy link
Member

@thoughtpolice thoughtpolice left a comment

lgtm

substituteInPlace ./CMakeLists.txt \
--replace 'git log -1 --format=%h' 'echo ${substring 0 11 (elemAt srcs 0).rev}'
Comment on lines -64 to -66

This comment has been minimized.

@thoughtpolice thoughtpolice merged commit fec5ada into NixOS:master Feb 9, 2020
15 checks passed
15 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
glasgow, icestorm, libfx2, nextpnr, symbiyosys, tinyprog, trellis, yosys on aarch64-linux Success
Details
glasgow, icestorm, libfx2, nextpnr, symbiyosys, tinyprog, trellis, yosys on x86_64-linux Success
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
@emilazy emilazy deleted the emilazy:update-eda-tools branch Feb 9, 2020
@sjmackenzie
Copy link
Contributor

@sjmackenzie sjmackenzie commented Feb 9, 2020

This PR has moved the nmigen package away from the official repository which is m-labs/nmigen [0]. cc @sbourdeauducq (you're on holiday atm), are you aware of this?

[0] - https://github.com/NixOS/nixpkgs/pull/79572/files#diff-9b7f0d79d5458a7f6e3466ed911bb97bR23

@sbourdeauducq
Copy link
Contributor

@sbourdeauducq sbourdeauducq commented Feb 9, 2020

Indeed, the official nmigen repositories are:
https://github.com/m-labs/nmigen
https://github.com/m-labs/nmigen-boards
https://github.com/m-labs/nmigen-soc

The homepage can be https://nmigen.org (currently simply redirects to the repository but we'll make one later).

@emilazy
Copy link
Member Author

@emilazy emilazy commented Feb 9, 2020

sigh I was hoping this issue would not come up here too...

For context, @sbourdeauducq is the developer of nMigen's predecessor Migen, and CEO of M-Labs, whitequark's former employer. The nmigen/nmigen repository is the repository where active development happens and is controlled by nMigen's lead and almost sole developer (@whitequark, see https://github.com/nmigen/nmigen/graphs/contributors). whitequark is also the sole controller of the PyPI packages, which the nixpkgs packages should move over to once there is an official stable release there. Currently, the only change in the m-labs repositories, other than being several days out of date, is to remove the credit for whitequark's current employer and the current funder of nMigen development. You can also see a long and unpleasant Twitter thread about this issue.

Obviously, the decision is up to those with nixpkgs commit access, but I do not believe this request is being made in good faith, and as the sole maintainer of these packages I am definitely uninterested in pointing them to a disgruntled former employer's clearly-downstream repository. Indeed, it would have been impossible to; at the time of this pull request, the current versions in m-labs' repository did not build together.

Sébastien – my sincerest hope is that you can reflect on what people are telling you about this situation and act in a less toxically passive-aggressive manner towards your (ex-)employees; it would be better for yourself, for M-Labs, and for the EDA community as a whole. I realize that GitHub is not the appropriate place for life advice, but nor is it the appropriate place for you to continue your vendetta against your ex-employee.

To the nixpkgs maintainers – sorry that this interpersonal dispute has reached your shores.

@sbourdeauducq
Copy link
Contributor

@sbourdeauducq sbourdeauducq commented Feb 10, 2020

sigh I was hoping this issue would not come up here too...
[...]
nor is it the appropriate place for you to continue your vendetta against your ex-employee.

Who started it by slipping in a change of upstream into a large pull request? Who is being passive-aggressive?

remove the credit for whitequark's current employer and the current funder of nMigen development.

Since you are bringing that up, I have to mention that - in addition to the several other arguments I made in the Twitter thread you linked - whitequark obtained that alleged funding behind my back while she was a M-Labs employee and we were trying to figure out ways to get her paid for nMigen work. I do not know under what terms she obtained it (employment or otherwise); as indicated in the commit message I have received no reply when I contacted them about it (still to this day).
I have no issue with the funding in itself - in fact on several occasions M-Labs matched donations made to nMigen (in addition to other funding and funding agreements) - simply with the underhanded way in which whitequark acted.

@emilazy
Copy link
Member Author

@emilazy emilazy commented Feb 10, 2020

This is hardly the first roll-up update PR of the EDA tools I've done: #66596, #67022, #67827, #68107, #71238. That's what maintaining packages is about, after all. In this case, I was getting around to fixing the issues with the Trellis package due to relevant upstream changes and a friend requesting I package nmigen-soc. This also involved making upstream changes to nmigen-soc and glasgow to get everything to compile and work properly.

You might get more replies to your communications if you didn't treat everyone like this. Are these really the actions of a heartbroken man (content warning: misgendering), or just a manipulative and controlling one?

I don't really have anything more to say on on this sorry topic, and no real power, so I'll leave this up to @thoughtpolice.

@thoughtpolice
Copy link
Member

@thoughtpolice thoughtpolice commented Feb 11, 2020

Who started it by slipping in a change of upstream into a large pull request?

The maintainer of the packages in question did -- someone who has complete authority to do so, to be clear -- and someone who has repeatedly shown good faith and work, time and time again, when it comes to contributing to this project. That's who did it actually. (The only reason Emily even goes through me at this point is formality, honestly.)

On top of that, there's me, the person who approved it. And as a long-time contributor and one of the maintainers of this part of Nixpkgs, I'm not exactly unfamiliar with the projects in question, their developers, their users, or who's who -- the FOSS EDA community isn't exactly large you know. So when I get a pull request from a co-maintainer to update a package and when I'm familiar with who the developers of that package are, it's very easy for me to see a change in upstream and know it's essentially 100%-without-any-doubt being made in good faith. Even then the reasoning is relatively easy to verify independently of even what Emily might say -- for instance, by looking at the Git history of nMigen or simply asking anyone involved about the change. Again, not a big community! What, did you expect this whole change/shitstorm was impossible to unravel? I'm not exactly Sherlock Holmes but I'm pretty sure this isn't a murder mystery and I could have figured it out.

There is also the angle about the employment of whitequark but, frankly, it's just none of my business. It means absolutely nothing to me, and it is not a game winning argument to allege betrayal or whatnot (nor does it give you a blank check to start shit over it; hell, even assuming someone did screw you over with 100% proof, it's still in your best interest not to talk about it as a representative of a corporation.)

The change in upstream repository did not slip by me as an afterthought; I reviewed this request as asked. But upstreams change all the time for a variety of good reasons. Had you or your friend not come in here make a big deal out of it (in a most fantastically poor form, I might add), I would have been completely under the impression that this is a normal organization transferral and none the wiser. Clearly, that is not the case. And unfortunately, making a big deal out of it isn't working out for you, obviously. Why on earth would I appease any individual or organization that would engage in this behavior? It's only an invitation for it to continue happening. No thanks.

Any discussion concerning a change/revert in upstream isn't continuing, because the change isn't being undone, nor is it up for any further debate (and I'll ensure I'm CC'd on any possible future change to these packages.) You're free to continue this discussion with me privately if you wish -- but it's probably not going to be very enjoyable, I'm afraid.

@NixOS NixOS locked as resolved and limited conversation to collaborators Feb 11, 2020
@NixOS NixOS unlocked this conversation Feb 12, 2020
@edolstra
Copy link
Member

@edolstra edolstra commented Feb 12, 2020

IMHO locking this issue is a bit premature. While we don't have an official policy on this (or much of anything, really), Nixpkgs does generally stick to upstream versions of packages. If forks are widely considered useful to have, then those can also be included in Nixpkgs, but they should have a different name. In any case the PR should have mentioned that it switches nmigen to a different repo.

@emilazy
Copy link
Member Author

@emilazy emilazy commented Feb 12, 2020

The nmigen/nmigen repository is the upstream version of the package in the minds of everyone but sb0. To maintain this illusion he has even modified other people's comments to edit out references to it being the upstream.

@Th3Fanbus
Copy link

@Th3Fanbus Th3Fanbus commented Feb 12, 2020

Uh, interesting. I never knew editing others' comments was a thing on GitHub.

uhm

@jonringer
Copy link
Contributor

@jonringer jonringer commented Feb 12, 2020

if you have commit access, you can. Generally, people don't.

@marcan
Copy link

@marcan marcan commented Feb 12, 2020

"Upstream" generally refers to the fork where active development takes place; there is no magical policy in open source that "upstream" is whoever claims the name first. "Upstream" can change organically, due to slow changes in the development team. It can also change instantaneously, such as when the entirety of the active development team switches repos, which was the case here. Regardless of who controls a GitHub or legal entity, it is entirely evident that active development of nmigen has instantaneously switched to this repo.

If anything it is now up to the m-labs folks to prove that their fork deserves to be canonical upstream by, say, taking it in a different/better/more active direction. Which I highly doubt will happen.

@thoughtpolice
Copy link
Member

@thoughtpolice thoughtpolice commented Feb 12, 2020

IMHO locking this issue is a bit premature.

No it isn't -- there's nothing left to discuss.

(or much of anything, really)

Then I'm not sure why you're second guessing me on a non-existent "policy" when I've got more than enough information to assess the situation, and direct knowledge of the involved tools and people. If you have an issue with the way I handled this, you can take it up with me directly -- personally -- but there's no "policy" to enforce here, and so any further debate isn't necessary, or fruitful.

If forks are widely considered useful to have, then those can also be included in Nixpkgs, but they should have a different name.

Unless you're directly taking the stance that the packages get renamed or something -- which absolutely, 100% is not going to happen, btw, and I would absolutely revert any change that did so no matter who authored it -- I'm not sure what the point of re-opening this is, other than to draw this out.

Software development is largely a social process, and sometimes you get situations like these. I don't know how much patience you have for behavior where your fellow maintainers are practically hunted down and abused because the annoying CEO of a company is obsessed enough with a stupid grudge to shoot their mouth off about private relationships with prior employees to win an internet argument (a ridiculous and flagrant ethical issue, I'd add) -- because I have absolutely zero patience for it. Frankly, you should have zero patience for it. Not only because it's abusive, but because there are few technical mistakes you could ever commit to Nixpkgs that are as bad as allowing that behavior, at all, on this issue tracker. And, besides, this change isn't a mistake in any technical sense -- it's the right choice, abusive interlopers aside.

I would make this same call on any package I was familiar with in the tree, and even ones I wasn't familiar with, provided someone was being abused similarly -- totally ignoring the fact this case is extremely clear cut technically.

Nixpkgs does generally stick to upstream versions of packages.

It's not that simple. Upstream is not a technical, immutable property of a repository in the same sense "this crayon is blue, this one is red". In this case the new repository is not only being actively developed by the primary maintainers (who was also doing development for a long time on at the old URL), and they own the namespace on systems like pypi. I'd argue actually it probably violates users intuition if they installed nmigen from nixpkgs and did not get the new repository -- especially since it will be used in shipping products.

If forks are widely considered useful to have, then those can also be included in Nixpkgs, but they should have a different name.

This argument isn't relevant though, because it's not a fork, it's the actual new upstream. This misunderstanding seems to be the root of the issue, so I hope that clears it up.

@sbourdeauducq
Copy link
Contributor

@sbourdeauducq sbourdeauducq commented Feb 12, 2020

Software development is largely a social process, and sometimes you get situations like these. I don't know how much patience you have for behavior where your fellow maintainers are practically hunted down and abused because the annoying CEO of a company is obsessed enough with a stupid grudge to shoot their mouth off about private relationships with prior employees to win an internet argument (a ridiculous and flagrant ethical issue, I'd add) -- because I have absolutely zero patience for it.

I don't see an ethical issue with calling out the bad behavior of former employees (or anyone else really) especially when it is relevant to explaining the issue at hand. You are simply trying to find excuses to silence me because the truth I am speaking offends you.

abusive interlopers aside.

Surely you are aware that the "abusive interloper":

  • designed and wrote much of the original Migen, from which nMigen is largely inspired
  • came up with several nMigen-specific ideas even if he did not implement them himself. There is more to it than the lines of code in the GitHub statistics.
  • provided financial and administrative support to whitequark
  • hired someone to work on nmigen-stdio and nmigen-soc
  • encouraged Lambdaconcept and others to use and sponsor nMigen

Not bad for an interloper, heh?

@emilazy
Copy link
Member Author

@emilazy emilazy commented Feb 12, 2020

...

Honestly, all I have to say at this point is that if movie-villain-esque speeches are how these things are decided then I'll have no desire to maintain the packages I do any more and will remove myself from maintainer-list.nix. I signed up to keep free software working and up to date, not to deal with... whatever this is.

Contributing to projects that can't recognize bad-faith actors is simply not worth the strain that having to deal with abusive politicking like this causes; I think that opinion will be shared by many existing and prospective NixOS contributors.

@sbourdeauducq
Copy link
Contributor

@sbourdeauducq sbourdeauducq commented Feb 12, 2020

Alternatively, you could try to negociate with me for me to recognize the new upstream (contrary to what you keep repeating, I am acting in good faith), but defamation and other abusive bullshit isn't going to work.

@emilazy
Copy link
Member Author

@emilazy emilazy commented Feb 12, 2020

Alternatively, you could try to negociate with me for me to recognize the new upstream

I think your desire for control above all else is beyond obvious to all observers at this point. ¯\_(ツ)_/¯

@thoughtpolice
Copy link
Member

@thoughtpolice thoughtpolice commented Feb 12, 2020

I don't see an ethical issue with...

Yes, that much is clear.

You are simply trying to find excuses to silence me because the truth I am speaking offends you.

You can clutch at straws all you want, but let me be clear: this bug tracker is for Nixpkgs relevant discussions, not for you and your surrogates mysteriously appear and harass maintainers because you've decided they "started" something with you, and think you can get away with it.

Eelco, if you'd like to discuss this further, please contact me directly, but do not unlock this issue. There's little point in this issue being open, and nothing is going to change here regardless (neither derivation name or upstream).

@NixOS NixOS locked as resolved and limited conversation to collaborators Feb 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

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