Skip to content

Enable fpm to create a new package #102

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

Conversation

everythingfunctional
Copy link
Member

This is just the bare minimum I would say is necessary for fpm new.

Copy link
Member

@LKedward LKedward left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good and works on my machine 👍

A minor observation: I expected the behaviour to be to initialise a project in the current directory (like git init), as opposed to the command creating a new directory - but I think that is just something that needs documenting somewhere for users.

Copy link
Member

@milancurcic milancurcic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I approve but this also needs to be documented in the README or elsewhere. What does it do exactly? Give an example.

A description in the PR body would help reviewers as well.

@milancurcic
Copy link
Member

A minor observation: I expected the behaviour to be to initialise a project in the current directory (like git init), as opposed to the command creating a new directory - but I think that is just something that needs documenting somewhere for users.

This (fpm new) behaviour is analogous to cargo new, and what you describe is analogous to cargo init. So if we wanted to have this, it would be covered by fpm init.

Co-authored-by: Laurence Kedward <[email protected]>
@everythingfunctional
Copy link
Member Author

Yeah, I'll add some documentation to the README, and maybe PACKAGING.md

@certik
Copy link
Member

certik commented Jun 19, 2020 via email

@milancurcic
Copy link
Member

@certik, see cargo new and cargo init.

In short, cargo new (and likewise, fpm new) creates a new directory and initializes the TOML and starter source files. I think this is what this PR implements, but again, without a PR description or some example in the documentation, hard to say. @everythingfunctional can you please describe the behavior with a small example? How is this invoked? What files are created?

cargo init initializes the package in an existing directory with existing source files. So fpm init will be used to "fpm-ize" an existing package that obeys the fpm layout. fpm new is used to bootstrap a new empty package. They're different.

At least this is my understanding of it.

@milancurcic
Copy link
Member

@everythingfunctional can you please describe the behavior with a small example? How is this invoked? What files are created?

Sorry, I didn't mean to nag. :) I saw your earlier response after writing this.

@certik
Copy link
Member

certik commented Jun 19, 2020

No, cargo init creates a new package in the current directory and cargo init b creates a new package in a new directory b. I am not sure what the difference is to cargo new. But I've only used cargo init and it creates a new package from scratch.

@certik
Copy link
Member

certik commented Jun 19, 2020

See #96 where I go into all the details. See also #96 (comment).

@certik
Copy link
Member

certik commented Jun 19, 2020

Both cargo new and cargo init seem identical to me. They both initialize in you current directory, or in a new directory if you provide it as an argument. The initialized files seem identical. What is the difference between the two?

@milancurcic
Copy link
Member

cargo new only creates a new directory and initializes a project in it.

cargo init initializes a project in an existing directory, and optionally does what cargo new does.

For cargo new, PATH is required. For cargo init, PATH is optional.

Indeed, if starting from scratch, they appear very much the same and redundant.

However, if there is anything special about the directory layout (multiple binaries, tests etc.) and contents that would be reflected in Cargo.toml, I would expect cargo init to do that. But I don't think it does, at least in the simple example I tried.

For fpm it should. For example, if you have a package layout with multiple programs, fpm init should be able to output a correct fpm.toml according to the special layout.

fpm new simply bootstraps something from scratch.

It's very nice and useful to use Cargo and Rust for reference, but it's not an end all. We're not targeting Rust users. What we make should make sense to us. fpm new for a new project very much makes sense, and fpm init makes more sense for initializing an existing project.

@certik
Copy link
Member

certik commented Jun 19, 2020 via email

@milancurcic
Copy link
Member

milancurcic commented Jun 19, 2020

For me it makes sense to use "fpm init" like "git init". Create a new project from scratch.

git init actually does not only create a new project from scratch. It initializes existing files in the directory into the git repository if there are any. It does also create a new git repository. I think it's difficult to argue that the word "init" is more intuitive for creating something new than the word "new". To me "init" means initialize, which means enable some capability in existing directory+files. In the context of fpm, that would mean simply create fpm.toml given existing pakage layout.

But I agree that we shouldn't have redundant commands like Cargo does. It looks like the choice is whether we want to have one command that does multiple things or two narrowly-scoped commands.

  1. We implement just fpm init which can both create a new empty project and "fpm-ize" an existing non-fpm package (basically generate a correct fpm.toml);
  2. We implement fpm new that creates an empty project and fpm init to initialize an existing package.

I think @certik prefers option 1. I prefer option 2. as it's easier to understand to me. There may be some other options I missed. @everythingfunctional @LKedward what do you prefer?

@milancurcic
Copy link
Member

An argument for fpm init over fpm new (all else being equal) is that init is a verb and new is a noun. Using a verb would be more consistent with our existing commands (build, install, run, test), and semantically makes more sense (do this with that).

So I am not opposed to the word "init". But I do prefer smaller utility commands over more general ones that would behave differently depending on current directory.

@everythingfunctional
Copy link
Member Author

Technically, giti init doesn't create a new project from scratch. It initializes an existing project's git configuration.

I like having two different commands because they do two different things. fpm new for new projects, and fpm init for existing projects.

Since the most common use case would be to start a new project from scratch, that's why I implemented it first.

@@ -53,6 +53,19 @@ stack install

On Linux, the above command installs `fpm` to `${HOME}/.local/bin`.

### Creating a new project

Creating a new fpm project is as simple as running the command `fpm new project_name`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Creating a new fpm project is as simple as running the command `fpm new project_name`.
Creating a new fpm project is as simple as running the command `fpm new <project-name>`.

This will create a new folder in your current directory with the following contents
and initialized as a git repository.

* `fpm.toml` with your project's name and some default standard meta-data
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* `fpm.toml` with your project's name and some default standard meta-data
* `fpm.toml` with your project's name and some default standard metadata

* `fpm.toml` with your project's name and some default standard meta-data
* `README.md` with your project's name
* `.gitgnore`
* `src/project_name.f90` with a simple hello world subroutine
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* `src/project_name.f90` with a simple hello world subroutine
* `src/<project-name>.f90` with a module containing a simple hello world subroutine

Comment on lines +66 to +67
* `app/main.f90` (if `--with-executable` flag used) a program that calls the subroutine
* `test/main.f90` (if `--with-test` flag used) an empty test program
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be tackled in a separate PR, but I suggest we use --bin instead of --with-executable (and also make it the default). Also, add an empty test program by default, as this will encourage all new projects to have tests.

@certik
Copy link
Member

certik commented Jun 19, 2020 via email

@milancurcic
Copy link
Member

How often do you expect to initialize fpm for an existing project? I don't ever expect to do that, after I convert my existing projects.

I expect that we'll use fpm init quite often in the early days, perhaps even more often than fpm new, as we'll work hard on adapting existing Fortran packages (ours and those of others) into fpm. In the long run, I expect fpm new would be used more often.

It also depends how you're used to doing things. I never ever ran git init to create a new repo from scratch. I always run it after I've written and played with some code and want to make a git repo out of it. Same with fpm. For any more complex layout package, I'd much rather run fpm init and have fpm write the fpm.toml for me, than me editing it by hand every time I add a new file.

Even if it's an edge case and something that we'd want down the road, I think it's useful to carefully design and name things early on.

@certik
Copy link
Member

certik commented Jun 19, 2020 via email

@certik
Copy link
Member

certik commented Jun 19, 2020 via email

@everythingfunctional
Copy link
Member Author

I honestly didn't even know that git init directory was a valid command. I learned something new today! I always thought you just had to git init inside a directory, existing files or not.

I still think that the command that makes the most sense for creating a new project is fpm new, even if that's not what Cargo does. Although I will note the following output from cargo, which would seem to suggest that cargo somewhat agrees with me. cargo new is for a new cargo package and cargo init is for in an existing directory.

[darter:~] cargo help new
cargo-new 
Create a new cargo package at <path>

USAGE:
    cargo new [OPTIONS] <path>
[darter:~] cargo help init
cargo-init 
Create a new cargo package in an existing directory

USAGE:
    cargo init [OPTIONS] [--] [path]

@certik
Copy link
Member

certik commented Jun 20, 2020

Here is the guide for Cargo:

https://doc.rust-lang.org/cargo/guide/creating-a-new-project.html

it seems they recommend cargo new also.

I explained my position above, Milan and Brad explained theirs, and it looks like they prefer to use fpm new instead of fpm init to start a new package. So unless more people have an opinion right now, let's go with fpm new and move on.

However, I reserve the right to run a poll later on, and if more people prefer fpm init, let's reconsider. Until then, let's do fpm new.

@milancurcic
Copy link
Member

However, I reserve the right to run a poll later on, and if more people prefer fpm init, let's reconsider. Until then, let's do fpm new.

Definitely, we don't have a majority agreement right now.

@everythingfunctional everythingfunctional merged commit 572bd33 into fortran-lang:master Jul 1, 2020
@everythingfunctional everythingfunctional deleted the CreateNewCommand branch July 8, 2020 18:03
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.

4 participants