Do not try to check if api.odocl exists.

samoht commented 7 years ago

When not using ocamlbuild, it doesn't.

samoht commented 7 years ago

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

dbuenzli commented 7 years ago

Could you please explain me:

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

samoht commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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

dbuenzli commented 7 years ago

@samoht are you still interested in this ?

samoht commented 7 years ago

yes :-)

samoht commented 7 years ago

ping?

samoht commented 6 years ago

ping?

dbuenzli commented 6 years ago

The patch is ok for me.

dbuenzli commented 6 years ago

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

dbuenzli commented 6 years ago

Thanks your patch is in as ceaa8ed753efc197ca2

dbuenzli / topkg