Pull request for NEW

[urbanjost]

sort out branches

add test program for CLI

fix fpm.toml version reference

remove --usage references from help text

basic RUN subcommand

remove dash from executable name to see if it clears MSWindows build error

try one more like previous build to clear error

build on proposed CLI interface to make a "new" and "run" subcommand for discussion

basic new,test,run added to build subcommands

change quoting of -- ARGS values for more platform independence and change test accordingly

replace cli_test test

consistent indenting

errata for NEW PR

remove doc

[certik]

There is some failure on Windows. Otherwise this looks good.

[urbanjost]

All tests pass now. I have built, run, and tested 17 packages built with h-fpm with this version of f-fpm. I think the changes to the fpm_command_line.f90 are self-contained and straight-forward to review, and well as the "new" subcommand. I just utilized the existing functionality in "build" to implement "run" and "test". The --list extension, the alternate --lib --test --app for --with-executable and --with-test and whether they should replace the --with-* options and what the default should be for "fpm new" and the new routine to find default test files are the main parts I am looking for affirmation on. The build/search/dependency is great and I am trying some Fortran/C projects that required fpm.mk files look like they will work with the new code; quite significant compared to what I hope are relatively minor changes I am proposing here. I look forward to everyone's review. Everyone has come a long way towards making a package that can just require gfortran and git and maybe ar and libcurl or OS-specific equivalents for ar. Although the main focus here is to make a package manager I am starting to get excited that this can also be used on a stand-alone system for a new Fortran programmer. It would be nice for a novice to not have to learn Make/CMake/... and ar and what switches to use on a compiler for debugging and production before they can gt very far with Fortran; and still be useable by someone that does know those things. Lots of things to resolve but I am getting excited that the day I want to make a program with command-line cracking, date and time functions, regular expressions, ... and a plethora of mathematical functions could be trivial. Historically Fortran programmers don't play well together. I have seen a lot of activity lately that shows this is changing and fpm(1) and stdlib really does seem like they could be the missing pieces that finally change this (only took sixty years).

[awvwgk]

The fpm.f90 gets a bit overloaded by this PR, I think it would be easier to create own modules for each run type and only have a procedure call in the toplevel module instead.

[awvwgk]

Any reason to use stop here and error stop in the other places?

[urbanjost]

The original skeleton had error stop, and there is some advantage to using an exit code to allow for unit testing. For gfortran an error stop also produces a traceback which can be useful for development but since gfortran and some other compilers produce a distracting "STOP" message it is very ugly for an interactive utility. In a production release I would recommend a standard stop everywhere except where an unexpected stop is provided. Unfortunately Fortran does not provide an equivalent to exit() that just sets the status. If you use the program interactively it only takes one stop error or a few stop N uses to find the resulting output is very distracting. A traceback can be a great tool for development but seems a bit hysterical for typical events that have been described with an error message already. So at this point I would recommend a custom stop function be called with a number that can later be turned off or only activated in test mode unless an exit(3c) is added to the stdlib. So a stop with no value was intentional. Would you consider a non-zero exit critical?

[urbanjost]

Basically the error stop was already being used in the original skeleton I started with and I have no strong preference on how stop is used except I hate there being no way to surpress the "STOP" message in gfortran (that varies from compiler to compiler). If anyone feels we need a rule like "error stop" for unexpected internal errors; all stop should have a numeric value, etc. let me know and I think it should then be in the style guide. Why on earth does the standard not allow for a stop with a string and a value where the string can be null? The differences between compilers regarding what output is produced by STOP is irrirating. And an explicit traceback or abort routine is preferable to leaving that up to each implementor too.

[milancurcic]

Thanks a lot, @urbanjost, I will play with it today.

[certik]

Can you please use 4 spaces to be consistent? Or 2 if you must, but based on our survey, almost nobody prefers 3 spaces.

[urbanjost]

Interesting. The most common indent I see I and I have many millions of lines of code is three; and quite a few auto-formatters defaulted to three last I looked. I did not see many votes on that so I think the sample size was too small. In general even python which uses indenting to control logic flow does not care about indenting except that it is there or is not. I liked the conclusion I that that came to that code could be run through a specific auto-indenter but have not seen that emerge. Changing the indenting would cause a lot of changes just based on white-space which other arguments here say is undesirable so I am torn here. If I run the whole thing through findent(1) for example it triggers a lot of changes here. I thought there were options in git to ignore white-space differences but must be mistaken. I was going to suggest that be turned on but I guess that was a false memory. Has there been any progress on an automatic reformatter? I saw the discussion on LFortran possibly doing that. Since I assume that would be based on a fully parsed source that could be much more flexible than most typical reformatters that usually work on a line-by-line basis with some basic split/join capacity.

[urbanjost]

Instead of dealing with the white-space issues piece-meal I ran the fpm.f90 and fpm_command_line.f90 files through a formatter for a one-time
(hopefully) pass to get it consistent, as there have been multiple authors and styles used in these files. The other changes were applied except for resolving if there is a benefit or standard for use of the STOP statement. In a prototype there are pros and cons to what a STOP statement does.

[everythingfunctional]

My opinion on the stop and error stop statements is that error stop should be reserved for things that would be indicative of a programming/logic bug. I.e. in the case default of a select case block. These bugs would then (hopefully) be found in development and testing, and the stack trace could be useful.

For any other errors the procedure should not stop the program. It should return the error to the caller in some way. Exactly how and in what form, and how sophisticated that is can be open for discussion, but it should probably be done consistently throughout the project.

This is so we can unit test such procedures. They're expected to encounter these errors (or you wouldn't have written the code to check for them in the first place) and therefore should be tested to handle those errors properly.

There are basically two designs one could use for this:

1. any procedure that does error handling has an intent(out) argument for returning any error(s) or (for functions) the return type is one that can contain the error(s)
2. any procedure that does error handling has an intent(in) argument of class(error_handler_t) with error handling TBPs that can be "mocked" in tests to prevent the unit tests from actually stopping and ensure that the appropriate error handling procedure was in fact called.

I generally prefer option 1, as all my procedures can then be pure, but it does have the drawback of sometimes leading to "noisy" code as one must always explicitly check the return value for errors.

[everythingfunctional]

Also FYI, from the standard:

According to this document, the following are processor dependent:
...
whether the processor supports a concept of process exit status, and if so, the process exit status on program
termination (5.3.7)

So there is no "standard" way of specifying the exit status of a program, and thus one should not rely on it. Obviously a tool like fpm should endeavor to provide a meaningful exit status, but our code and tests shouldn't rely on it.

[milancurcic]

@everythingfunctional Minor nit-pick: There is a standard way do it. it's stop [code] and error stop [code] (Section 11.4). What the standard doesn't promise is whether the OS or compiler support process exit codes (exactly your quote). But they can be set/specified in a standard-conforming way. So the question is really whether the OSs we target support process exit codes. I think they do and this could be safely relied on. Otherwise I agree with your recommendation to return the error to the caller rather than stopping.

(Edit: This may very well be what you meant and if so, please ignore my nit-pick :))

[everythingfunctional]

It's an important subtle detail that was worth pointing out. It doesn't really change the recommendation though, limit the places that stop is explicitly called to a sectioned off, explicit error handling part of the code so the rest of the codebase doesn't rely on it.

[milancurcic]

Thank you for the hard work @urbanjost. I played with it and am overall happy with the fpm new behavior. I very much appreciate your addition of fpm <command> --help outputs. I also did a cursory read through of the new code. Here are my suggestions:

• In src/fpm_command_line.f90, rename fortran_name() to is_fortran_name(). Rationale: The function returns a logical, so the function name should reflect that.
• fpm --help output ends with "displayed help text". I assume "displayed help text" is a temporary output for development, so let's remove it.
• Also, fpm --help ends with "STOP 1". I think fpm --help should return normally (from the end program) statement, and not from stop 1, as recommended by the standard. In other words fpm --help should not trigger an erroneous exit.
• fpm --version outputs something like:

$ fpm --version
VERSION:     0.1.0, Pre-alpha
PROGRAM:     fpm(1)
DESCRIPTION: A Fortran package manager and build system
HOME PAGE:   https://github.com/fortran-lang/fpm
LICENSE:     MIT

displayed version text
STOP 3

I find the key words a bit loud. Can we make them lowercase? Also, fpm --version should not return with a non-zero code IMO. Also, flip the order of Program and Version, so finally we get:

$ ffpm --version
Version:     0.1.0, Pre-alpha
Program:     fpm(1)
Description: A Fortran package manager and build system
Home page:   https://github.com/fortran-lang/fpm
License:     MIT

I think this is all for now. If there's more I will write, but otherwise I'm mostly happy with this PR. We can smooth out the rough corners in future PRs.

[milancurcic]

Regarding the indentation level discussion, I don't really care which indentation width is most common in the wild. What I care the most is consistency with existing code in this project. So I think a good rule of thumb is, if you're adding code to a module, just follow the style of that module (be it 2 or 4 spaces). I appreciate you making the indentation changes so the new code is consistent with the old. If you're starting a new module, sure, try to be consistent with the other modules, but I doubt anybody would get upset over 3-spaces. After all, 3 spaces are the closest to being most consistent with both 2 and 4 spaces. :)

In fpm we currently have a mix of 2- and 4-space indentation, and a mix of indent vs. don't indent the program unit bodies. It's okay. These things will sort themselves out naturally in the long run, and if they don't, it just means it doesn't matter all that much. In the meantime, we simply make a common sense best effort to be mostly consistent.

[urbanjost]

This should bring it up to date except for items that I think should be left as separate PRs and issues. The refactoring of fpm.f90 into individual pieces seems worth looking at as the run and test subcommands are fleshed out, but the original scope was to implement the new subcommand. The build command has progressed rapidly in the meantime and feature creep is getting to the point where I think it is easier to make the refactoring a separate PR so the other commands remain in sync; after which there are a good number of issues with new and the CLI that can be addressed much more cleanly. A more flexible treatment of STOP throughout the project and perhaps the stdlib project is a bigger scope than was intended here and deserves an issue of its own. I think this addresses everything else specific to new that should not be discussed as a separate issue first.
Obviously this depends on everyone's review and after looking at other package managers I have several further proposals for *new myself.

[LKedward]

Looks great @urbanjost! I agree with you that other changes are best left for separate PRs or until after we have discussed them.
I notice that there are several auxiliary doc/ files now in this PR, including binary and html - is this intentional?

[urbanjost]

The doc/ directory can be deleted. My old personal build system has some automatic document building capabilities that convert files ending in .man to manpages using txt2man(1) and then use groff(1) or man2html(1) and tidy(1) to generate html. I was running ccall and h-fpm and f-fpm on all the packages listed in the fpm registry and ccall triggered on the scratch *.man pages I made from testing the help. Those binary files are compressed *roff files which is the standard input for man(1).
Since they were there I thought they might be useful for reviwing the help text with nicer formatting but on second thought they are a confusion.

[milancurcic]

I agree, let's not add doc/ at this time and discuss documentation in a dedicated issue.

[urbanjost]

refreshed with a new pull and repaired that file and no more git errors. @awvwgk could you verify?

[milancurcic]

It builds and runs okay on my end.