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

hop: 2.5.1 → 3.3.0 #97887

Merged
merged 2 commits into from Sep 22, 2020
Merged

hop: 2.5.1 → 3.3.0 #97887

merged 2 commits into from Sep 22, 2020

Conversation

vbgl
Copy link
Contributor

@vbgl vbgl commented Sep 13, 2020

This fixes the build of Hop and then updates it to the most recent version (along with the tightly coupled bigloo compiler).

Motivation for this change

Hop is currently broken.

ZHF: #97479

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.

cc @thoughtpolice maintainer of bigloo

@ofborg ofborg bot requested a review from thoughtpolice September 13, 2020 06:30
risicle
risicle reviewed Sep 13, 2020
View reviewed changes
pkgs/development/compilers/hop/default.nix Outdated
configureFlags = [
"--bigloo=${bigloo}/bin/bigloo"
"--bigloolibdir=${bigloo}/lib/bigloo/4.1a/"
"--bigloolibdir=${bigloo}/lib/bigloo/4.3h/"
Copy link
Contributor

@risicle risicle Sep 13, 2020 •
edited

Choose a reason for hiding this comment

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

You might be able to lower the future maintenance burden here by using bigloo.name and builtins.parseDrvName to extract the correct version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Parsing a name in order to guess an install directory seems fishy. Maintaining fishy code may not qualify for lowering the maintenance burden.

Thanks nonetheless for the suggestion.

Copy link
Contributor

Choose a reason for hiding this comment

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

Dude, extracting the version number from a package's name is the standard way of doing it (that's why there's a parseDrvName function in builtins) and we "guess" directory names from version numbers every time we use python.sitePackages (everywhere) - but you're the maintainer...

@risicle
Copy link
Contributor

risicle commented Sep 13, 2020

Fails to build for me on macos 10.14:

@nix { "action": "setPhase", "phase": "unpackPhase" }
unpacking sources
unpacking source archive /nix/store/b98wxml7lx1ysd0cd4r1a5nswka9bg0n-bigloo-4.3h.tar.gz
source root is bigloo-4.3h
setting SOURCE_DATE_EPOCH to timestamp 1589265422 of file bigloo-4.3h/bglpkg/configure.scm
@nix { "action": "setPhase", "phase": "patchPhase" }
patching sources
@nix { "action": "setPhase", "phase": "configurePhase" }
configuring
configure flags: --prefix=/nix/store/c9r8inq2jn2z8218pp1fq9fqh5gjn0bf-bigloo-4.3h
Checking local configuration...
executing ./autoconf/os -> darwin
executing ./autoconf/cpu -> x86_64
executing ./autoconf/osversion -> 18.7.0
executing ./autoconf/user -> nixbld1
executing ./autoconf/cctest -> clang
executing ./autoconf/alignment -> 3
executing ./autoconf/macosx -> yes
* MacOS X tuning enabled
executing ./autoconf/ascpp -> clang -c -x assembler-with-cpp
executing ./autoconf/arflags -> qc
executing ./autoconf/ranlib -> yes
executing ./autoconf/strip -> true
executing ./autoconf/thread -> -D_THREAD_SAFE -D_REENTRANT -DGC_THREADS -DFINALIZE_ON_DEMAND
executing ./autoconf/pthread -> -lpthread
executing ./autoconf/pthreadlock -> 0
executing ./autoconf/pthreadspinlock -> 0
executing ./autoconf/pthreadmutexrec -> 1
executing ./autoconf/pthreadcondvtimedwait -> 0
executing ./autoconf/pthreadtimedjoin -> 0
executing ./autoconf/pthreadlocalstorage -> 
executing ./autoconf/pthreadkill -> PTHREAD_KILL
executing ./autoconf/nbprocs -> 8
executing ./autoconf/semaphore -> 1
executing ./autoconf/lockf -> 1
executing ./autoconf/gcflags -> -DHAS_X86_THREAD_STATE64___RAX
executing ./autoconf/maemo.arm -> no
+ Installing custom GC (gc-8.0.4) source code.
executing ./autoconf/config.guess -> --host=x86_64-apple-darwin18.7.0 --build=x86_64-apple-darwin18.7.0
+ Configuring custom GC (gc-8.0.4)...
Configuring custom multi-threaded GC...
configure: WARNING: "Explicit GC_INIT() calls may be required."
executing ./autoconf/gcversion -> 804

Preparing and configuring custom gmp (gmp/gmp-6.1.2.tgz)...
executing ./autoconf/configfsf.guess -> --host=haswell-apple-darwin18.7.0 --build=haswell-apple-darwin18.7.0

+ Installing custom libuv (v1.20.3) source code.
+ Configuring custom libuv...
+ glibtoolize --copy
./autogen.sh: line 43: glibtoolize: command not found
executing ./autoconf/config.guess -> 
./configure-libuv: line 21: ./configure: No such file or directory
configure libuv fail, see configure.log file.

@zimbatm
Copy link
Member

zimbatm commented Sep 13, 2020

Result of nixpkgs-review pr 97887 1

2 packages built:
  • bigloo
  • hop

@vbgl
Copy link
Contributor Author

vbgl commented Sep 13, 2020

Thanks for testing! The macOS issue looks like an upstream bug to me… I’ve just pushed a fix (I hope, as I cannot test).

@risicle
Copy link
Contributor

risicle commented Sep 13, 2020

The darwin fix is definitely a step closer - just think it needs one more addition which I'm now testing...

@risicle
Copy link
Contributor

risicle commented Sep 13, 2020

Hmm, no, I haven't quite got to the bottom of it. I do know you've got to provide darwin.apple_sdk.frameworks.ApplicationServices to bigloo to get it a step further, however the sticking point is during the final link of the bigloo executable it fails to find its own bundled libunistring and pcre (which it did manage to successfully build). Strange.

@vbgl
Copy link
Contributor Author

vbgl commented Sep 15, 2020

I’ve just added the missing dependency of bigloo for macOS. Please let me know if you understand the linking issue.

@risicle
Copy link
Contributor

risicle commented Sep 16, 2020

I ... think I lack the macos-fu to figure this one out unfortunately.

@vbgl
hop: fix build
d538645 
The configure process tries to escape the sandbox by writing into /tmp
@vbgl
bigloo: 4.1a-2 → 4.3h; hop: 2.5.1 → 3.3.0
fbd0b7a
jonringer
jonringer approved these changes Sep 22, 2020
View reviewed changes
Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

LGTM

Result of nixpkgs-review pr 97887 1

2 packages built:
  • bigloo
  • hop

@jonringer jonringer merged commit 06098c7 into NixOS:master Sep 22, 2020
@vbgl vbgl deleted the bigloo-4.3h branch September 22, 2020 14:26
@vbgl
Copy link
Contributor Author

vbgl commented Sep 22, 2020

Merged the hop fix into 20.09 as 6e31319

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

@risicle risicle risicle left review comments

@jonringer jonringer jonringer approved these changes

@thoughtpolice thoughtpolice Awaiting requested review from thoughtpolice

Assignees
No one assigned
Labels
8.has: package (update) 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 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
@vbgl @risicle @zimbatm @jonringer