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

Do not try to check if api.odocl exists. #114

Closed
wants to merge 1 commit into from

Conversation

samoht
Copy link
Contributor

@samoht samoht commented Jun 20, 2017

When not using ocamlbuild, it doesn't.

@samoht
Copy link
Contributor Author

samoht commented Jun 29, 2017

@dbuenzli? Any opinion on that PR? Seems to work fine for me so far :-)

@dbuenzli
Copy link
Owner

Could you please explain me:

  1. What the problem is.
  2. How this solves the problem.
  3. What changes for users of ocamlbuild.

@samoht
Copy link
Contributor Author

samoht commented Jun 29, 2017

What the problem is.

The current way to use topkg for release together with jbuilder for builds is to use https://github.com/diml/topkg-jbuilder. The package defines a custombuild step which simply calls jbuilder build @install for mostly everything, and jbuilder build @doc when it guess docs are being built. It is not the most beautiful piece of code ever, but it works.

jbuilder has its own rules to call odoc so it doesn't care about the doc/api.odocl file. However, topkg is checking that this file exists before calling the defined build rules. So if that file doesn't exists topkg doc fails but jbuilder build @doc would have worked perfectly fine.

How this solves the problem.

I removed the check for that file.

What changes for users of ocamlbuild.

If the file doesn't exist, the error message is a bit less readable:

Solver failed:
  Ocamlbuild knows of no rules that apply to a target named doc/api.odocl. This can happen if you ask Ocamlbuild to build a target with the wrong extension (e.g. .opt instead of .native) or if the source files live in directories that have not been specified as include directories.
Backtrace:
  - Failed to build the target doc/api.docdir/index.html
      - Building doc/api.docdir/index.html:
          - Building doc/api.odocl
pkg.ml: [ERROR] cmd ['ocamlbuild' '-use-ocamlfind' '-classic-display' '-tag' 'debug'

@dbuenzli
Copy link
Owner

Thanks for the answers. I'd be willing to merge this at a certain point. But @diml told me yesterday he was looking for a strategy to detect jbuilder in topkg and better jbuilder interaction so I'd rather wait for his results before piling up the hacks.

@samoht
Copy link
Contributor Author

samoht commented Jun 29, 2017

That makes sense. Looking forward to see @diml's updates :-)

@dbuenzli
Copy link
Owner

dbuenzli commented Aug 8, 2017

@samoht are you still interested in this ?

@samoht
Copy link
Contributor Author

samoht commented Aug 8, 2017

yes :-)

@samoht
Copy link
Contributor Author

samoht commented Sep 4, 2017

ping?

@samoht
Copy link
Contributor Author

samoht commented Oct 10, 2017

ping?

@dbuenzli
Copy link
Owner

The patch is ok for me.

@dbuenzli
Copy link
Owner

(I need to allocate myself a bit of time to do a release, will try in the upcoming 7 days)

@dbuenzli
Copy link
Owner

Thanks your patch is in as ceaa8ed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants