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

Jump to bottom

termbox: 1.1.2 -> 1.1.3 #99581

Closed
wants to merge 1 commit into from
Closed

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.

hugolgst
hugolgst approved these changes Oct 4, 2020
View reviewed changes
Copy link
Member

@hugolgst hugolgst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

Result of nixpkgs-review pr 99581:

3 packages built:
mle nimmm termbox

@ofborg ofborg bot requested a review from fgaz October 4, 2020 20:00
@fgaz
Copy link
Member

fgaz commented Oct 5, 2020

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

@fgaz
Copy link
Member

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 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 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

fgaz
fgaz requested changes Oct 6, 2020
View reviewed changes
pkgs/development/libraries/termbox/default.nix Outdated
@@ -1,31 +1,22 @@
{ stdenv, fetchFromGitHub, python3, wafHook, fetchpatch }:
{ stdenv, fetchFromGitHub, fetchpatch }:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fetchpatch shouldn't be needed anymore

@adsr
termbox: 1.1.2 -> 1.1.3
da20a16 
Repointing repo to termbox/termbox as nsf/termbox is no
longer maintained.
@adsr
Copy link
Contributor Author

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 October 7, 2020 15:28
fgaz
fgaz approved these changes Oct 7, 2020
View reviewed changes
Copy link
Member

@fgaz fgaz left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 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 commented Oct 26, 2020

I have no power to do that, sorry

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

@SuperSandro2000
Copy link
Member

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

3 packages built:
  • mle
  • nimmm
  • termbox

@SuperSandro2000
Copy link
Member

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 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
@adsr
Attempt fix build on macOS
4235e57
@SuperSandro2000
Copy link
Member

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.

I don't have the time to test this. Can you just mark it broken on darwin?

@adsr
Copy link
Contributor Author

adsr commented Jan 2, 2021

Build passes on a fastmac vm[1]

https://github.com/fastai/fastmac

@SuperSandro2000

This comment has been minimized.

Sign in to view
@SuperSandro2000
Copy link
Member

This is a semi-automatic executed nixpkgs-review which does not build all packages (e.g. lumo, tensorflow or pytorch)
If you find some bugs or got suggestions for further things to search or run please reach out to SuperSandro2000 on IRC.

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

3 packages built:
  • mle
  • nimmm
  • termbox

@SuperSandro2000
Copy link
Member

darwin:

builder for '/nix/store/z2hb4y38h0iaym72wy21hma4pq3kpmci-termbox-1.1.3.drv' failed with exit code 2; last 10 log lines:                                                                       no configure script, doing nothing                                                                                                                                                          building                                                                                                                                                                                    build flags: SHELL=/nix/store/4n2ah3plbfnfz14wrzmz1pkfw78y9kz4-bash-4.4-p23/bin/bash prefix=/nix/store/frn8mfax71715zlipw8zxchy9ka20lh2-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

@fgaz
Copy link
Member

fgaz commented Feb 17, 2021

I see that 1.1.4 is out

@adsr
Copy link
Contributor Author

adsr commented Feb 20, 2021

Superseded by #113831

@adsr adsr closed this Feb 20, 2021
@cho-m cho-m mentioned this pull request Apr 23, 2022
Closed
46 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fgaz fgaz fgaz approved these changes

@hugolgst hugolgst hugolgst approved these changes

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

Successfully merging this pull request may close these issues.

None yet
4 participants
@adsr @fgaz @SuperSandro2000 @hugolgst