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.

Sign up for GitHub

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

Jump to bottom

kermit: fix build #107658

Merged
merged 1 commit into from Dec 26, 2020
Merged

kermit: fix build #107658

merged 1 commit into from Dec 26, 2020

Conversation

blitz
Copy link
Contributor

@blitz blitz commented Dec 26, 2020 •
edited

... by explicitly selecting the Linux code paths in the source. This is the solution that other people have also suggested, e.g. in the Arch community: https://bugs.archlinux.org/task/62918

I've tested kermit with a USB serial adapter and it works.

Motivation for this change

Kermit is still useful when interacting with boot loaders and other serial communication.

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.

@blitz
kermit: fix build
7bd7b5b 
... by explicitly selecting the Linux code paths in the source.
@blitz blitz requested review from Ma27 and pSub December 26, 2020 19:40
@SuperSandro2000
Copy link
Member

Result of nixpkgs-review pr 107658 run on x86_64-linux 1

1 package built:
  • kermit

Please do not overwrite patchPhase but use postPatch instead.

@pSub pSub merged commit 9e5bb15 into NixOS:master Dec 26, 2020
@SuperSandro2000
Copy link
Member

@pSub Why did you merge this? I left a review commit which should be addressed.

@Ma27
Copy link
Member

Ma27 commented Dec 26, 2020

But that wasn't even part of the diff in question, right?

@SuperSandro2000
Copy link
Member

But that wasn't even part of the diff in question, right?

No but while we are at it we can also fix this.

@blitz
Copy link
Contributor Author

blitz commented Dec 27, 2020

@SuperSandro2000 I'll create a follow-up PR with cleanups.

@blitz blitz mentioned this pull request Dec 27, 2020
Merged
10 tasks
@pSub
Copy link
Member

pSub commented Dec 27, 2020

@Ma27 Sorry. I didn't intend to merge this with an open review comment. However, even now I am unable to find your review. The status on this ticket is stimm "Awating requested review from Ma27".

@Ma27
Copy link
Member

Ma27 commented Dec 27, 2020

@SuperSandro2000 mentioned this in their first comment, not me :)

With my question I wanted to point out though that this patch fixed an actual problem, so I don't really understand why the review process should be slowed down. Filing another PR (if one cares enough) isn't that much of an effort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ma27 Ma27 Awaiting requested review from Ma27

@pSub pSub Awaiting requested review from pSub

Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 1-10 10.rebuild-linux: 1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@blitz @SuperSandro2000 @Ma27 @pSub