ocsigen / ocsigen-start

Ocsigen-start: Higher-level library to develop Web and mobile applications with users, (pre)registration, notifications, etc.
Other
75 stars 32 forks source link

4.0.0 : trouble when compiling webapp (ocsigen-i18n-rewriter command) #619

Open MdeLv opened 3 years ago

MdeLv commented 3 years ago

debian 10 ocaml 4.10.0 ocsigen-start.4.0.0

psql startup is fine. make test.byte immediately triggers the following error:

mkdir _deps                                                                                              
LC_ALL=C ocsigen-i18n-generator \                                                                        
--languages en,fr \                                                                                      
--default-language en \                                                                                  
< assets/test400_i18n.tsv \                                                                               
> test400_i18n.eliom                                                                                      
LC_ALL=C ocsigen-i18n-generator \                                                                        
--languages en,fr \                                                                                      
--default-language en \                                                                                  
--primary test400_i18n.tsv \                                                                              
< assets/test400_Demo_i18n.tsv \                                                                          
> test400_Demo_i18n.eliom                                                                                 
eliomdep -server -ppx -ppx "ocsigen-i18n-rewriter --prefix 'test400_' --suffix '_i18n' --default-module Te
st400_i18n" -package lwt_ppx -package js_of_ocaml-ppx_deriving_json -package ppx_deriving.std -package pgo
caml -package ocsigen-start.server test400_page.eliomi > _deps/test400_page.eliomi.server.tmp && mv _deps/t
est400_page.eliomi.server.tmp _deps/test400_page.eliomi.server                                             
Stdlib.Arg.Bad("Ocsigen-i18n-rewriter: unknown option '--default-module'.\ni18n_ppx_rewrite.native [OPTIO
NS] DEFAULT_MODULE\n  --prefix The prefix added to module names\n  --suffix The suffix added to module na
mes\n  -help  Display this list of options\n  --help  Display this list of options\n")                   
File "test400_page.eliomi", line 1:                                                                       
Error: Error while running external preprocessor                                                         
Command line: ocsigen-i18n-rewriter --prefix 'Test400_' --suffix '_i18n' --default-module Test400_i18n '/tm
p/camlppx29667b' '/tmp/camlppx69b47c'                                                                    

Makefile.os:306: .depend: No such file or directory                                                      
make: *** [Makefile.os:325: _deps/test400_page.eliomi.server] Error 2                                     
rm test400_Demo_i18n.eliom                                                                                

It seems related to the non existent --default-module option for ocsigen-i18n-rewriter command. But I don't know ocsigen-i18n-rewriter , and its help is limited:

$ ocsigen-i18n-rewriter --help                        
Usage: /home/test/.opam/4.10.0/bin/ocsigen-i18n-rewriter [extra_args] <infile> <outfile>

Do you have any idea about how to fix that?

jrochel commented 3 years ago

Thanks for the report. The issue was a dependency with a too lax version constraint (https://github.com/ocsigen/ocsigen-start/commit/58d345a2df34cab9d179284a319341e2ee854a1d). To fix your issue you can simply reinstall the latest version of ocsigen-i18n: opam update && opam install ocsigen-i18n.3.7.0.

A fixed version of ocsigen-start will be released shortly: https://github.com/ocaml/opam-repository/pull/18030

Feel free to close this issue if upgrading ocsigen-i18n resolves your problem.

MdeLv commented 3 years ago

ocsigen-i18n.3.7.0 was already installed as specified since at least ocsigen-start.2.21.1 See line 22 https://github.com/ocsigen/ocsigen-start/commit/58d345a2df34cab9d179284a319341e2ee854a1d#diff-2a240532b2d070b7c414d09a4fb3865388ebc66571d43578501fa7f352f03c9fR21-L22 (opam seems to take the last specification for ocsigen-i18n).

image

In Makefile.i18n, the option--default-language is used and triggers an error when ocsigen-i18n-rewriter is called. `I18N_PPXREWRITER := "ocsigen-i18n-rewriter --prefix 'Test92' --suffix '_i18n' --default-language Test92_i18n"

Looking at i18n_ppx_rewriter.ml, I see that val default_module_name is defined, but the related option --default-language is undefined in val options_spec (line 76):

let options_spec =
  ["--prefix", Arg.Set_string module_prefix, "The prefix added to module names"
  ;"--suffix", Arg.Set_string module_suffix, "The suffix added to module names"]

The solution is:

let options_spec =
  ["--prefix", Arg.Set_string module_prefix, "The prefix added to module names"
  ;"--suffix", Arg.Set_string module_suffix, "The suffix added to module names"
  ;"--default-module", Arg.Set_string default_module_name, "The default language"]

See PR https://github.com/besport/ocsigen-i18n/pull/43. I let you fix the opam file.

clembu commented 3 years ago

it's --default-module not --default-language and is introduced in the dune branch of https://github.com/besport/ocsigen-i18n

the change to using it is due to an oversight in the testing of the template.

Until the dune branch of ocsigen-i18n is deployed, we should retro-fit the template to be compatible with the 3.7.0 version

clembu commented 3 years ago

i opened a PR to fix the template. @MdeLv (and anyone encountering the same issue) in the meantime you can change your Makefile.i18n to remove the --default-module mention. That module argument currently doesn't need a label.

MdeLv commented 3 years ago

I erroneously mentionned --default-module" in my previous post while the PR uses --default-language which is indeed used in Makefile.i18n . So, the/one fix (that works) is:

;"--suffix", Arg.Set_string module_suffix, "The suffix added to module names"
;"--default-language", Arg.Set_string default_module_name, "The default language"]

I understand from your post that --default-module must be used everywhere instead of --default-language. I tested your fix. It works. Thanks.

About the name of values: let default_module_name = ref "" https://github.com/besport/ocsigen-i18n/blob/7be46cbb1805a8a4f2f01feaae66c1af26268314/i18n_ppx_rewriter.ml#L24

I understand what is --default-language in the context of i18n. I don't understand the meaning of --default-module. Can you explain it?

clembu commented 3 years ago

the --default-module argument (anonymous in the current version) is the name of the module used by default for the rewriter.

ocsigen-i18n allows to have multiple i18n files. Can be useful for several use cases. We use that to separate wordings by themes and uses (so general menuing is not in the same file as wordings for user profile pages for example). The rewriter will then allow you to write [%i18n Feature.wording] and it will rewrite it to {--prefix}Feature{--suffix}.Tr.wording ()

It uses the --default-module to know what to module to call to when not specifying any module: [%i18n wording] would go to {--default-module}.Tr.wording ()

jrochel commented 2 years ago

@clembu Do we need better documentation in the README?

clembu commented 2 years ago

OS's readme? or os-i18n?

jrochel commented 2 years ago

Good question. Ocsigen-i18n I'd say. I looked at the README but I'm not sure whether I extract what you've written above from it.

clembu commented 2 years ago

Hmm. I'll revise it in the weekend

MdeLv commented 1 year ago

Revised?