-init is now taken into account when -e is used

[EmileTrotignon]

Right now, with init.ml

let a = "hello"

Running ocaml -init init.ml works; it launches a toplevel where print_endline a works as expected.

However running ocaml -init init.ml -e "print_endline a;;" give Unbound value a.
And ocaml -init doesnotexist.ml -e "print_endline a;;" also gives Unbound value a and no other error.

This PR fixes that behaviour by reading the init argument before running a script.

It is a one line change, but code order is changed in order for the extra function to be available.

[EmileTrotignon]

This also fixes ocaml -init a.ml b.ml which was previously broken.

[gasche]

What happens with .ocamlinit files?

It is a one line change, but code order is changed in order for the extra function to be available.

You could have a first commit that just move the code around, and then a commit for the change.

Note: I notice that -e is not documented in man ocaml, this is something to be fixed as well.

[EmileTrotignon]

What happens with .ocamlinit files?
They are always read which is what the doc seems to imply. If its an issue for backwards compatibility its probably possible to introduce a difference between explicitly passing -init and just having a .ocamlinit file.

Another issue I have not been able to untangle this afternoon is that ocaml -blabla a.ml results in an error, while ocaml a.ml -blabla does not.

[EmileTrotignon]

The above also means that ocaml a.ml -init b.ml ignores the init file

[kit-ty-kate]

Another issue I have not been able to untangle this afternoon is that ocaml -blabla a.ml results in an error, while ocaml a.ml -blabla does not.

This is because the arguments after the file are expected to be the ones given to the script you're calling:

$ ocaml test.ml -a -b -c
test.ml
-a
-b
-c
$ cat test.ml
let () =
  for i = 0 to Array.length Sys.argv - 1 do
    print_endline Sys.argv.(i);
  done

However, annoyingly it isn't very well documented either. The man page only shows the following syntax:

ocaml [ options ] [ object-files ] [ script-file ]

I think it should probably be something like

ocaml [ options ] [ object-files ] [ script-file [argument...] ]

similar to the syntax from the ocamlrun man page.

[EmileTrotignon]

I pushed a commit that makes it so initialisation file lookup is disabled on scripts (including -stdin and -e). Passing -init still works in that commit.

And another commit that introduces -yesinit that allows to still lookup initialization files even when running scripts.

[dra27]

My intuition is that the semantics of -yesinit are potentially dangerous (in particular considering it would allow #!/usr/bin/ocaml -yesinit). It's one thing to configure your system to have default startup files for an interactive thing, but I'm not sure that ocaml as a script interpreter should ever be reading, even explicitly?

Regardless, the name doesn't work - the opposite of "no" is not "yes" here (the opposite of the instruction "process no init files" is not "process yes init files", if you see what I mean). Not sure what it ought to be, but perhaps something more like -with-init or even -run-init-scripts-yes-I-realise-it-could-be-bad

[EmileTrotignon]

My intuition is that the semantics of -yesinit are potentially dangerous (in particular considering it would allow #!/usr/bin/ocaml -yesinit).

This is not the main thing this PR aims to solve. I included it for the sake of completeness, but I'm happy to remove it.

Regardless, the name doesn't work

I am aware, I did not find anything better, and I thought it was understandable enough and at least a little funny. I thought the proper name would have been -init which is taken. -with-init is good.

[gasche]

cc @dbuenzli : you have experience running script through the toplevel, and may have valuable feedback on the desirable semantics with respect to initialization files.

[dbuenzli]

When you run scripts you generally want to be able to invoke it regardless of your cwd or personal configuration so I'd just drop that -yesinit which is very brittle. Here is the behaviour I would expect:

1. If ocaml is run to get an interactive REPL and no -init is specified, the implicit lookup for an rc file is performed ($PWD/.ocamlinit, $HOME/.ocamlinit, $XDG_CONFIG_HOME/ocaml/init.ml).
2. If ocaml is run to evaluate an expression (-e) or run a script, the implicit lookup is not be performed.
3. If ocaml is run with -init FILE, FILE is always read and replaces the implicit lookup if any. ocaml should error if FILE does not exist.

It is mostly what ocaml does at the moment except:

1. That -init is not read if -e is specified, which this PR fixes.
2. No error is reported if FILE does not exist with -e or script mode, which is bad.

[EmileTrotignon]

Do we have some sort of consensus that this can be merged when -yesinit is removed ?

@dbuenzli suggestion include more stuff, but that can be in another PR

[dra27]

For me, yes - in removing -yesinit, I think the patch will be simpler if run_script just calls use_silently directly if Clflags.init_file is Some file rather than the current re-plumbing to load_ocamlinit

[EmileTrotignon]

I have removed -yesinit, simplified the implementation according to @dra27 's comment and rebased on trunk.

[dra27]

You could do something cute with Option.fold (and so share the code for the ~some case), but that's certainly not compulsory!

Tweak to the man page, but this looks ok to me - however, can it have another pair of eyes ack it, just because toplevel startup is subtle, and (possibly I/)we've broken it in the past.

[ygrek]

should fail here, not just print

[dra27]

I thought this, too - but I think the current interpretation for an interactive toplevel doesn’t either?

[EmileTrotignon]

No it doesn't, I copy pasted the code from the toplevel part.

[ygrek]

yes, and it is unexpected^Wbug
inb4 https://xkcd.com/1172/

[dra27]

It’s been that way for a long time (since it was added in 2005 in df6f81e) - at the very least, it’s a separate issue (doesn’t look to me like @damiendoligez did that by mistake, although FWIW I wouldn’t have!)

[EmileTrotignon]

I agree completely this should be changed, but I think it should be done in another PR so that this one changes less things and is easier to merge. I will factor the code so that this line appears only once and make a PR that changes it.

[dra27]

LGTM (suggestion depends on how much one likes using Option functions - the other could be shortened if we had CCOption.or_lazy...)