Skip to content

Fix build of implementation only opamParserTypes in Makefile build#36

Closed
sternenseemann wants to merge 1 commit intoocaml:masterfrom
sternenseemann:makefile-impl-only
Closed

Fix build of implementation only opamParserTypes in Makefile build#36
sternenseemann wants to merge 1 commit intoocaml:masterfrom
sternenseemann:makefile-impl-only

Conversation

@sternenseemann
Copy link
Copy Markdown

@sternenseemann sternenseemann commented Dec 6, 2020

I had some issues linking against opam-file-format when it was built using the GNU make build process. I'm not sure if my solution is the best, but it has a pretty small diff and seems to work (I tested linking with building opaline).

[Having the make build process in place is kind of important for NixOS, since we'd otherwise run into circular dependencies since everything processing .install files depends on this library and we need to install it somehow which the Makefile nicely solves.]

@dra27
Copy link
Copy Markdown
Member

dra27 commented Dec 6, 2020

Thanks! This is an alternate fix to the one proposed in #35 - I think that renaming the .mli will be the best way, but not completely sure yet!

@dra27
Copy link
Copy Markdown
Member

dra27 commented Dec 6, 2020 •
edited
Loading

I’m intrigued by the circular dependency - doesn’t using dune install deal with that?

@sternenseemann
Copy link
Copy Markdown
Author

sternenseemann commented Dec 6, 2020

Thanks! This is an alternate fix to the one proposed in #35 - I think that renaming the .mli will be the best way, but not completely sure yet!

I've tried that, but I couldn't figure how to get it to work with the current build system (i. e. without touching the source), so I went with this solution.

I’m intrigued by the circular dependency - doesn’t using dune install deal with that?

I think it would, but we are using opaline for installing packages build by dune (and installing dune 1.x) which causes the circular dependency. I think I'll look into dune install soon-ish, since it is kind of annoying and I suspect the usage of opaline is for historical reasons.

This was referenced Dec 17, 2020
Merged
Merged
@dra27
Copy link
Copy Markdown
Member

dra27 commented Dec 17, 2020

This fix is in fact orthogonal to #35 - it does bring the Makefile build system inline with the current behaviour for Dune, but only inasmuch as they are then both equivalently broken. Compiling OpamParserTypes as a full module should fix this properly for both builds systems (and should be released this week)

@dra27 dra27 closed this Dec 17, 2020
@sternenseemann sternenseemann mentioned this pull request Dec 21, 2020
Merged
10 tasks
@sternenseemann sternenseemann mentioned this pull request Jan 9, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@sternenseemann @dra27