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

[bug] ppx_flags no longer allows passing arguments to ppxs #3709

Closed
jaredly opened this issue Jul 29, 2019 · 7 comments
Closed

[bug] ppx_flags no longer allows passing arguments to ppxs #3709

jaredly opened this issue Jul 29, 2019 · 7 comments

Comments

@jaredly
Copy link
Contributor

jaredly commented Jul 29, 2019

Not sure when this happened, but it's broken again 🙃
If I have "ppx-flags": ["./some-ppx --an-arg"] I get the error:

> bsb -make-world

ninja: error: rebuilding 'build.ninja': '/path/to/some-ppx --some-arg', needed by 'src/Hello.mlast', missing and no known rule to make it

Apparently ninja is being passed the ppx path and the argument as though it's a single file that is a dependency.

(bs-platform version 5.0.4)

@bobzhang
Copy link
Member

does this work for you:

"ppx-flags" : ["./some-ppx", "--an-arg"]

@bobzhang
Copy link
Member

The change is that recently we did some check to enforce that ppx binary actually exists for better error message

@jaredly
Copy link
Contributor Author

jaredly commented Jul 29, 2019

@bobzhang nope that doesn't work -- --an-arg is treated as a separate ppx

@bobzhang
Copy link
Member

bobzhang commented Jul 30, 2019

The sanity check was introduced since @rickyvetter found when ppx binary changes, the files won't get rebuilt. Since user could pass a list of ppxes, so the ideal config would be

[ ["ppx1", ...ppx1-args], ["ppx2", ... ppx2-args], ...]

I am going to relax such check. For your own temporary work around, since you own your own ppx, you can pass it without arguments.

I did not support ppx with arguments since it is really complicated to make it work with Windows etc.

bobzhang added a commit that referenced this issue Jul 30, 2019
@bobzhang
Copy link
Member

@jaredly I changed the schema to support ppx arguments, see this: e9f68ec

@bobzhang
Copy link
Member

merged in master

@jaredly
Copy link
Contributor Author

jaredly commented Aug 3, 2019

thanks!

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