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

Some problems with warning/error in findlib #19

Open
chetmurthy opened this issue Mar 16, 2021 · 4 comments
Open

Some problems with warning/error in findlib #19

chetmurthy opened this issue Mar 16, 2021 · 4 comments

Comments

@chetmurthy
Copy link
Contributor

chetmurthy commented Mar 16, 2021

Gerd, I'm finding some problems with findlib's warning/error. Here's an example:

ocamlfind ocamlc -verbose -package camlp5.pa_o,camlp5.pr_o -syntax camlp5r -c foo.ml  "
Effective set of preprocessor predicates: preprocessor,syntax,camlp5r
Effective set of compiler predicates: pkg_camlp5.pa_o,pkg_camlp5.pr_o,syntax,autolink,byte

In the META file for camlp5, I have:

package "pa_o" (
  error(camlp5r) = "camlp5.pa_o cannot be used with syntax camlp5r"
...
}

But this is useless, because error/warning are evaluated against compiler predicates, and "camlp5o" is a syntax predicate. I think that, in addition to adding "syntax" to syntax_preds and predicates, it would be useful to add syntax_<syntax> (where <syntax> is the specified syntax, e.g. syntax_camlp5o).

What do you think?

It's a small change, and I can send you a PR, but wanted to get your thoughts first.

P.S. Obviously I can make this change in chetmurthy/not-ocamlfind and it will suffice for many of my needs, but I figured, this is a useful consistency-check for findlib itself. It will make error/warning variables much more useful.

@gerdstolpmann
Copy link
Contributor

I think this is a nice idea, and I would like to merge such a PR.

There is the question whether (1) to keep the two predicate namespaces (where one space is mapped to the other), or (2) to merge the namespaces completely. In (1) you cannot set -predicate syntax_foo but only -syntax foo, and syntax_foo is allowed as selector only. In (2) -predicate syntax_foo would be seen as equivalent to -syntax foo.

I'm not sure which one is more consistent in the end. but I think (2) looks more promising.

@chetmurthy
Copy link
Contributor Author

chetmurthy commented Mar 16, 2021

Gerd,

Sure, why not merge them? Um, may I make an attempt at a PR which does these two things? That is, my original proposal, plus your #2 ?

ETA: The only problem I see, is that all packages that use syntax predicates will need to be updated. But this is a finite set, and I would expect that we can find them and update them. Maybe one could do it as follows (a staged approach):

(1) make my suggested change, but DO NOT remove syntax predicates. Instead, everywhere that syntax predicates is used, concat syntax_preds and predicates

This will allow all current packages to continue working, but allow to rewrite packages to the new way of doing things.

Release this.

(2) then we can go thru and fix every package that uses preprocessors

(3) and finally a new release that removes syntax_preds.

Thoughts?

@gerdstolpmann
Copy link
Contributor

Let's try (1) and gather experience.

@chetmurthy
Copy link
Contributor Author

chetmurthy commented Mar 16, 2021 via email

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

No branches or pull requests

2 participants