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

ocamlPackages.ocaml_extlib: 1.7.7 -> 1.7.8 #110374

Closed
wants to merge 1 commit into from

Conversation

anmonteiro
Copy link
Member

@anmonteiro anmonteiro commented Jan 21, 2021

Motivation for this change

4.12 support. see ocaml/opam-repository#17998 (comment)

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.

@sternenseemann
Copy link
Member

@GrahamcOfBorg build google-drive-ocamlfuse

@sternenseemann
Copy link
Member

@anmonteiro Seems like this beak google-drive-ocamlfuse as mentioned in the linked thread. This probably could be fixed by packaging extlib-compat and using it for google-drive-ocamlfuse.

@anmonteiro
Copy link
Member Author

Thanks, @sternenseemann. I'm OK waiting for the final deliberation in ocaml/opam-repository#17999 (comment) before this gets merged.

@kit-ty-kate
Copy link

The option the opam-repository took is to simply patch 1.7.7: ocaml/opam-repository#18041.
I'm not sure how NixOS deals with conflicts but since extlib and extlib-compat are incompatible it was not the right solution for us.

Furthermore the compat version adds more incompatible modules than previous version of extlib so I would suggest not to use it in NixOS.

@anmonteiro
Copy link
Member Author

Thanks, Kate! I think taking the same approach and using your patches here could work. I'll look into that later today.

@kit-ty-kate
Copy link

Note that the two patches I cherry-picked were taken from the upstream's master branch: ygrek/ocaml-extlib#55

@kvtb
Copy link
Contributor

kvtb commented Feb 10, 2021

haxe 4.0.x and 4.1.x need extlib 1.7.7 but haxe 4.2.0 needs extlib 1.7.8
it seems that we need both

@sternenseemann
Copy link
Member

haxe 4.0.x and 4.1.x need extlib 1.7.7 but haxe 4.2.0 needs extlib 1.7.8
it seems that we need both

Maybe the upstream patches would apply cleanly on the older versions? Then we wouldn't need to keep two extlib versions around.

@kvtb
Copy link
Contributor

kvtb commented Feb 10, 2021

haxe 4.0.x and 4.1.x need extlib 1.7.7 but haxe 4.2.0 needs extlib 1.7.8
it seems that we need both

Maybe the upstream patches would apply cleanly on the older versions? Then we wouldn't need to keep two extlib versions around.

Actually, I do not think older Haxe's are worth including in official nixpkgs, haxe 4.2.0 + extlib 1.7.8 is good enough.

I added all the intermediate versions to local nixpkgs just to bisect problems migrating from 3.4.x

@sternenseemann
Copy link
Member

Then we probably should do the haxe update in this PR as well.

@kvtb
Copy link
Contributor

kvtb commented Feb 10, 2021

Then we probably should do the haxe update in this PR as well.

#95638 (comment)

just remove extlib-1.7.7 and haxe versions which depend on it (4.0.x and 4.1.x)

@sternenseemann sternenseemann mentioned this pull request Mar 15, 2021
10 tasks
@sternenseemann
Copy link
Member

See also #116953.

@anmonteiro
Copy link
Member Author

closed in favor of #117113

@anmonteiro anmonteiro closed this Mar 21, 2021
@anmonteiro anmonteiro deleted the anmonteiro/extlib-1.7.8 branch March 21, 2021 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants