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

termbox: 1.1.2 -> 1.1.3 #99581

Open
wants to merge 1 commit into
base: master
from
Open

termbox: 1.1.2 -> 1.1.3 #99581

wants to merge 1 commit into from

Conversation

@adsr
Copy link
Contributor

@adsr adsr commented Oct 4, 2020

Repointing repo to termbox/termbox as nsf/termbox is no longer maintained.

Motivation for this change

Update version and repo location

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
Contributor

@hugolgst hugolgst left a comment

Looks good to me.

Result of nixpkgs-review pr 99581:

3 packages built:
mle nimmm termbox
@fgaz
Copy link
Member

@fgaz fgaz commented Oct 5, 2020

How do we know that's the new canonical/established location?

@fgaz
Copy link
Member

@fgaz fgaz commented Oct 5, 2020

I looked around a bit and

  • freebsd ports just moved it to the new repo: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=249672
  • termbox-haskell was moved under github.com/termbox by its author
  • the new termbox/termbox maintainer already contributed to he project before the fork and maintains a text editor based on termbox that's packaged on many distros

So I guess that's enough for trust, but it doesn't seem to be that established yet (for example of the two dependent packages in nixpkgs, only one points to the fork)

@adsr
Copy link
Contributor Author

@adsr adsr commented Oct 5, 2020

Hi @fgaz yes I am working to recentralize development at termbox/termbox and avoid fork hell. I updated FreeBSD last week and will be updating Alpine and Debian next. If you'd like I can send you a copy of the email I sent to the termbox community a few months ago.

@fgaz
Copy link
Member

@fgaz fgaz commented Oct 6, 2020

Hi @adsr, thanks for chiming in (and for your work on a new central source of course)!

If you'd like I can send you a copy of the email I sent to the termbox community a few months ago

If it isn't a problem please do

@@ -1,31 +1,22 @@
{ stdenv, fetchFromGitHub, python3, wafHook, fetchpatch }:
{ stdenv, fetchFromGitHub, fetchpatch }:

This comment has been minimized.

@fgaz

fgaz Oct 6, 2020
Member

fetchpatch shouldn't be needed anymore

Repointing repo to termbox/termbox as nsf/termbox is no
longer maintained.
@adsr adsr force-pushed the adsr:termbox-v1.1.3 branch from 7e31d08 to da20a16 Oct 7, 2020
@adsr
Copy link
Contributor Author

@adsr adsr commented Oct 7, 2020

@fgaz 👍 I forwarded a copy to the email address on your github profile.

Thanks for the review.

@ofborg ofborg bot requested a review from fgaz Oct 7, 2020
@fgaz
fgaz approved these changes Oct 7, 2020
Copy link
Member

@fgaz fgaz left a comment

LGTM

Also @adsr I didn't even realize you also authored this pr, so that's the reason for my previous comments' weirdness like using third person :-P

@adsr
Copy link
Contributor Author

@adsr adsr commented Oct 26, 2020

Hello. Can someone add a hacktoberfest-accepted label to this issue so that it counts toward Hacktoberfest?

@fgaz
Copy link
Member

@fgaz fgaz commented Oct 26, 2020

I have no power to do that, sorry

(But I do have some hacktoberfest-tagged repos 🙂)

@SuperSandro2000
Copy link
Contributor

@SuperSandro2000 SuperSandro2000 commented Nov 26, 2020

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

3 packages built:
  • mle
  • nimmm
  • termbox
@SuperSandro2000
Copy link
Contributor

@SuperSandro2000 SuperSandro2000 commented Nov 26, 2020

Fails to build on darwin:

these derivations will be built:
  /nix/store/qdh2rvwkm08sln5hq90wn21a3s8662ps-termbox-1.1.3.drv
building '/nix/store/qdh2rvwkm08sln5hq90wn21a3s8662ps-termbox-1.1.3.drv'...
unpacking sources
unpacking source archive /nix/store/nm569bqm38wnn9wdwl72ykwdk5jflrzc-source
source root is source
patching sources
configuring
no configure script, doing nothing
building
build flags: SHELL=/nix/store/k89nm2jva0qmvd970f84wq2iq1iwm9bs-bash-4.4-p23/bin/bash prefix=/nix/store/d4lqkg71vi739sg9b8a5ybfdrkljx33h-termbox-1.1.3
clang -c -std=c99 -Wall -Wextra -pedantic -Wno-unused-result -fPIC -g -O3 -D_XOPEN_SOURCE  termbox.c -o termbox.o
clang -c -std=c99 -Wall -Wextra -pedantic -Wno-unused-result -fPIC -g -O3 -D_XOPEN_SOURCE  utf8.c -o utf8.o
ar rcs libtermbox.a termbox.o utf8.o
clang -shared -Wl,-h,libtermbox.so.1 termbox.o utf8.o -o libtermbox.so.1.0.0
ld: unknown option: -h
clang-7: error: linker command failed with exit code 1 (use -v to see invocation)
make: *** [Makefile:20: libtermbox.so.1.0.0] Error 1
builder for '/nix/store/qdh2rvwkm08sln5hq90wn21a3s8662ps-termbox-1.1.3.drv' failed with exit code 2
error: build of '/nix/store/qdh2rvwkm08sln5hq90wn21a3s8662ps-termbox-1.1.3.drv' failed
@adsr
Copy link
Contributor Author

@adsr adsr commented Nov 27, 2020

Hello, can someone try building with this patch?

termbox/termbox@4235e57

I don't have a mac to test with. If it looks ok I can tag as 1.1.4 or add a patch here.

adsr referenced this pull request in termbox/termbox Nov 29, 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

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