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

Fix Makefile typo #10268

Closed
wants to merge 2 commits into from
Closed

Fix Makefile typo #10268

wants to merge 2 commits into from

Conversation

nojb
Copy link
Contributor

@nojb nojb commented Mar 3, 2021

Following #10169 (I think)

@nojb nojb requested a review from dra27 March 3, 2021 07:01
@nojb
Copy link
Contributor Author

nojb commented Mar 3, 2021

Sorry, this is the wrong fix. By the way I am trying to fix the following warnings (observed in OS X and also in the RISC-V CI at least):

+ make -j4 --warn-undefined-variables
stdlib/StdlibModules:56: warning: undefined variable ' '
stdlib/StdlibModules:56: warning: undefined variable ' '
stdlib/StdlibModules:56: warning: undefined variable ' '
stdlib/StdlibModules:56: warning: undefined variable ' '
stdlib/StdlibModules:56: warning: undefined variable ' '
stdlib/StdlibModules:56: warning: undefined variable ' '
stdlib/StdlibModules:56: warning: undefined variable ' '
stdlib/StdlibModules:56: warning: undefined variable ' '
stdlib/StdlibModules:56: warning: undefined variable ' '
stdlib/StdlibModules:56: warning: undefined variable ' '
stdlib/StdlibModules:56: warning: undefined variable ' '
stdlib/StdlibModules:56: warning: undefined variable ' '
stdlib/StdlibModules:56: warning: undefined variable ' '
stdlib/StdlibModules:56: warning: undefined variable ' '
stdlib/StdlibModules:56: warning: undefined variable ' '
stdlib/StdlibModules:56: warning: undefined variable ' '
...

@Octachron
Copy link
Member

My solution to please both check-typo and the undefined Makefile variable warning was to define a separate upper function:
https://github.com/ocaml/ocaml/blob/trunk/api_docgen/Makefile.docfiles#L22

@dra27
Copy link
Member

dra27 commented Mar 3, 2021

A variant of this works:

--- a/stdlib/StdlibModules
+++ b/stdlib/StdlibModules
@@ -47,6 +47,9 @@ STDLIB_PREFIXED_MODULES=\

 # add stdlib__ as prefix to a module except for internal modules
 # and the stdlib module itself
+EMPTY=
+SPACE=$(EMPTY) $(EMPTY)
+$(SPACE) :=
 define add_stdlib_prefix
   $(or $(filter-out $(STDLIB_PREFIXED_MODULES), $1), \
     stdlib__$(shell echo $1 | cut -c1 | tr '[:lower:]' '[:upper:]')$\

More shortly...

@shindere
Copy link
Contributor

shindere commented Mar 3, 2021 via email

@dra27
Copy link
Member

dra27 commented Mar 3, 2021

Ack - just testing an alternate fix

@nojb
Copy link
Contributor Author

nojb commented Mar 3, 2021

Closing this in favor of #10270

@nojb nojb closed this Mar 3, 2021
@nojb nojb deleted the makefile_typo branch March 3, 2021 19:55
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.

None yet

4 participants