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

Sections "Field Syntax Reference" and "Package Description" are out of sync and inconsistent #9186

Open
BinderDavid opened this issue Aug 18, 2023 · 6 comments

Comments

@BinderDavid
Copy link
Contributor

What is wrong with the docs?
There are two toplevel sections in the cabal users guide which document the cabal file format. Section 6 (Package description) and section 10 (Field Syntax Reference). Unfortunately, they contradict each other.

For example:

  • Field "default-language": Section 6 says that Haskell98 Haskell2010 and GHC2021 are allowed, and that the field was available since version 1.12. Section 10 says that Haskell98 and Haskell2010 are allowed, and that the field was available since version 1.10.
  • There are a lot of other fields where the two sections disagree on when the field was introduced.
  • In section 10 the license field is undocumented, so it is missing the crucial information that only SPDX identifiers are accepted in newer versions of the cabal file format.
  • The documentation of the cabal-version field in section 10 mentions a >=1.0 default, but version ranges are no longer allowed for newer versions of cabal, and there is no information that this is a special field which should occur only in the very first line.

In general, I have the impression that the documentation in section 6 is much more accurate and current than the documentation in section 10. I think the intention was for section 10 to contain a semi-formal BNF grammar for cabal files, but if this information is incomplete and out-of-sync, then the value of documenting every field twice is a bit dubious.

So what should be done here? If given some guidance on what would be the best approach here I would volunteer some time and could for example try to merge the information of both sections: Go through all the information in sec. 10 and add any information that is missing in sec. 6 to the documentation of the respective fields. But if there are better suggestions on what to do here I would also be thankful :)

@gbaz
Copy link
Collaborator

gbaz commented Aug 22, 2023

cf also #7545

In general, I think section 6 is where the meaningful information about the content of the fields should go. Anything relevant from section 10 should be moved there, and section 10 should be given a sentence at the top that explains it is an incomplete description of the basic raw syntax of cabal files, but semantics and more details are given elsewhere. IMHO, in general, we should keep the "since" and "deprecated" info in section 6, not 10, since if its going to live in one place, that will be the one most updated and most referred to.

@BinderDavid
Copy link
Contributor Author

cf also #7545

In general, I think section 6 is where the meaningful information about the content of the fields should go. Anything relevant from section 10 should be moved there, and section 10 should be given a sentence at the top that explains it is an incomplete description of the basic raw syntax of cabal files, but semantics and more details are given elsewhere.

Then I maybe still fail to see what the purpose of section 10 is, precisely? The section is currently written by hand, not generated from some parser specification, and incomplete (it contains quite a few TODOs). And the section 6 already contains formal BNF grammars for a lot of the fields. For example, the field mixin has two formal BNF grammars:

Both BNF grammars seem to be written by hand and are reasonably complicated, and I can not see at first glance if they agree or don't. Section 6 also contains a lot of "pseudo" BNF grammars like token list or compiler list or freeform etc as annotations to the fields. And if I am not mistaken, then token list in section 6 is isomorphic to {hs-string | [" "]^c+ }^* in section 10. What I am trying to say is that there is no clear distinction what the difference between section 6 and 10 should be. As you write, semantic information should be in section 6, but the syntactic specification also lives duplicated in section 6 and 10. So maybe section 10 does not serve any purpose at the moment and could be removed by merging it with section 6?

I think we could take some inspiration from the Coq user guide, which also uses the read-the-docs sphinx style: They add a hyperlinked BNF syntax of each tactic directly to the documentation of the tactic. E.g. https://coq.inria.fr/refman/proof-engine/tactics.html#controlling-the-proof-flow

@philderbeast
Copy link
Collaborator

philderbeast commented Dec 28, 2023

doc/buildinfo-fields-reference.rst is (or was) generated.

There's a cabal.project.buildinfo project (that I'm looking at for #9565) and a make recipe:

cabal/Makefile

Lines 60 to 64 in 52d1fb6

# generated docs
buildinfo-fields-reference : phony
cabal build --builddir=dist-newstyle-bi --project-file=cabal.project.buildinfo buildinfo-reference-generator
$$(cabal list-bin --builddir=dist-newstyle-bi buildinfo-reference-generator) buildinfo-reference-generator/template.zinza | tee $@

I've cobbled together some fixes locally to the generator (so that it builds) and its makefile recipe (using cabal run instead) and find that the generated file is not so different from doc/buildinfo-fields-reference.rst:

diff -u './doc/buildinfo-fields-reference.rst' './doc/buildinfo-fields-reference-cobbled-together.rst'
--- ./doc/buildinfo-fields-reference.rst	2023-12-15 09:44:38.263261635 -0500
+++ ./doc/buildinfo-fields-reference-cobbled-together.rst	2023-12-28 07:14:46.314239915 -0500
@@ -338,6 +338,7 @@
 
 extra-lib-dirs-static
     * Monoidal field
+    * Available since ``cabal-version: 3.8``.
     * Documentation of :pkg-field:`extra-lib-dirs-static`
 
     .. math::
@@ -350,16 +351,17 @@
     .. math::
         \mathrm{commalist}\left\{ \mathop{\mathit{hs\text{-}string}}\mid{{[\mathop{\mathord{``}\mathtt{\ }\mathord{"}}\mathop{\mathord{``}\mathtt{\text{,}}\mathord{"}}]^c}}^+_{} \right\}
 
-extra-library-flavours
+extra-libraries-static
     * Monoidal field
-    * Documentation of :pkg-field:`extra-library-flavours`
+    * Available since ``cabal-version: 3.8``.
+    * Documentation of :pkg-field:`extra-libraries-static`
 
     .. math::
         \mathrm{commalist}\left\{ \mathop{\mathit{hs\text{-}string}}\mid{{[\mathop{\mathord{``}\mathtt{\ }\mathord{"}}\mathop{\mathord{``}\mathtt{\text{,}}\mathord{"}}]^c}}^+_{} \right\}
 
-extra-libraries
+extra-library-flavours
     * Monoidal field
-    * Documentation of :pkg-field:`extra-libraries-static`
+    * Documentation of :pkg-field:`extra-library-flavours`
 
     .. math::
         \mathrm{commalist}\left\{ \mathop{\mathit{hs\text{-}string}}\mid{{[\mathop{\mathord{``}\mathtt{\ }\mathord{"}}\mathop{\mathord{``}\mathtt{\text{,}}\mathord{"}}]^c}}^+_{} \right\}
@@ -428,6 +430,14 @@
     .. math::
         \mathrm{optcommalist}\left\{ \mathop{\mathit{hs\text{-}string}}\mid{{[\mathop{\mathord{``}\mathtt{\ }\mathord{"}}\mathop{\mathord{``}\mathtt{\text{,}}\mathord{"}}]^c}}^+_{} \right\}
 
+hsc2hs-options
+    * Monoidal field
+    * Available since ``cabal-version: 3.6``.
+    * Documentation of :pkg-field:`hsc2hs-options`
+
+    .. math::
+        {\left\{ \mathop{\mathit{hs\text{-}string}}\mid{{[\mathop{\mathord{``}\mathtt{\ }\mathord{"}}]^c}}^+_{} \right\}}^\ast_{\bullet}
+
 include-dirs
     * Monoidal field
     * Documentation of :pkg-field:`include-dirs`
@@ -473,7 +483,6 @@
 
 other-extensions
     * Monoidal field
-    * Available since ``cabal-version: 1.10``.
     * Documentation of :pkg-field:`other-extensions`
 
     .. math::
@@ -504,23 +513,12 @@
 virtual-modules
     * Monoidal field
     * Available since ``cabal-version: 2.2``.
-    * Documentation of :pkg-field:`library:virtual-modules`
+    * Documentation of :pkg-field:`virtual-modules`
 
     .. math::
         \mathrm{commalist}\left({\left(\mathop{\mathit{upper}}{\left\{ \mathop{\mathit{alpha\text{-}num}}\mid[\mathop{\mathord{``}\mathtt{\text{'}}\mathord{"}}\mathop{\mathord{``}\mathtt{\text{_}}\mathord{"}}] \right\}}^\ast_{}\right)}^+_{\mathop{\mathord{``}\mathtt{\text{.}}\mathord{"}}}\right)
 
 
-Library fields
---------------
-
-visibility
-    * Optional field
-    * Documentation of :pkg-field:`library:visibility`
-
-    .. math::
-        \left\{ \mathop{\mathord{``}\mathtt{public}\mathord{"}}\mid\mathop{\mathord{``}\mathtt{private}\mathord{"}} \right\}
-
-
 Package description fields
 --------------------------
 
@@ -557,11 +555,11 @@
 
 data-dir
     * Optional field
-    * Default: ``""``
+    * Default: ``.``
     * Documentation of :pkg-field:`data-dir`
 
     .. math::
-        \left\{ \mathop{\mathit{hs\text{-}string}}\mid{{[\mathop{\mathord{``}\mathtt{\ }\mathord{"}}\mathop{\mathord{``}\mathtt{\text{,}}\mathord{"}}]^c}}^+_{} \right\}
+        \mathsf{\color{red}{TODO}}
 
 data-files
     * Monoidal field
@@ -612,7 +610,7 @@
     * Documentation of :pkg-field:`license-file`
 
     .. math::
-        \mathrm{optcommalist}\left\{ \mathop{\mathit{hs\text{-}string}}\mid{{[\mathop{\mathord{``}\mathtt{\ }\mathord{"}}\mathop{\mathord{``}\mathtt{\text{,}}\mathord{"}}]^c}}^+_{} \right\}
+        \mathsf{\color{red}{TODO}}
 
 maintainer
     * Free text field
@@ -655,6 +653,14 @@
 Test-suite fields
 -----------------
 
+code-generators
+    * Monoidal field
+    * Available since ``cabal-version: 3.8``.
+    * Documentation of :pkg-field:`test-suite:code-generators`
+
+    .. math::
+        \mathrm{commalist}\left\{ \mathop{\mathit{hs\text{-}string}}\mid{{[\mathop{\mathord{``}\mathtt{\ }\mathord{"}}\mathop{\mathord{``}\mathtt{\text{,}}\mathord{"}}]^c}}^+_{} \right\}
+
 main-is
     * Optional field
     * Documentation of :pkg-field:`test-suite:main-is`
@@ -675,3 +681,5 @@
 
     .. math::
         \left\{ \mathop{\mathord{``}\mathtt{exitcode\text{-}stdio\text{-}1\text{.}0}\mathord{"}}\mid\mathop{\mathord{``}\mathtt{detailed\text{-}0\text{.}9}\mathord{"}} \right\}
+
+

@Mikolaj
Copy link
Member

Mikolaj commented Dec 28, 2023

Wow, that's a surprise. We have to add this to the release checklist. How often, do you think, the docs were supposed to be regenerated? Once per major release? Minor? Every time anything relevant changes (and commited in the same PR)?

@philderbeast
Copy link
Collaborator

I made a stab at picking up relevant changes with:

doc/buildinfo-fields-reference.rst : \
  $(wildcard Cabal-syntax/src/*/*.hs Cabal-syntax/src/*/*/*.hs Cabal-syntax/src/*/*/*/*.hs) \
  $(wildcard Cabal-described/src/Distribution/Described.hs Cabal-described/src/Distribution/Utils/*.hs) \
  buildinfo-reference-generator/src/Main.hs \
  buildinfo-reference-generator/template.zinza
	cabal build buildinfo-reference-generator
	cabal run buildinfo-reference-generator buildinfo-reference-generator/template.zinza | tee $@

@ulysses4ever
Copy link
Collaborator

Every release, I think (major and minor). This definitely should be added on the release checklist. Until, that is, people decide to merge the two sections or whatever. I personally think that two big sections is better than one humongous but I can see arguments both ways.

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

No branches or pull requests

5 participants