add clean command

[freevryheid]

Only tested on linux.

Closes #454

[ivan-pi]

Nice addition. In a few situations I recall deleting the build folder manually to get things work again (it might have been due to some clashes after upgrading fpm).

[certik]

Instead of asking at a prompt, why not to also add fpm clean -f that would simply delete it without asking?

[ivan-pi]

Would it make sense to reuse the build subcommand for this purpose? E.g. in git, you make a new branch with git branch <branch_name> and you delete it with git branch -d <branch_name>.

Following this line of reasoning, we could perhaps have

• fpm build --d, delete with prompt confirmation
• fpm build --D, forced delete (or --df perhaps)

In any case, I think it's worth some debate whether this should be a new subcommand or a subcommand option.

[epagone]

Would it be too complicated to have both?

I think those are two distinct, typical use cases:

• you want to "clean" and then decide what to do (e.g. follow-up with fpm run or fpm test, etc...)
• you know already you want to re-build or re-run and then you'll use the flags -d or -D

[ivan-pi]

I fail to see how those two cases are distinct unless you mean fpm build -D would first delete the folder and then re-build afterward?

My motivation to comment was primarily because introducing a new subcommand should not be taken lightly. The sub-command clean could potentially be given a different role, e.g

• removing unused variables and other semantically meaningful source code transformations that lead to "clean" code,
• prettifying/linting (indentation, casing, etc.)
• removing unused packages or other superfluous entries from the package manifest

I do like the analogy between fpm clean and the common make clean rule, in the sense that this command serves to return your project directory into it's initial state.

[epagone]

I fail to see how those two cases are distinct unless you mean fpm build -D would first delete the folder and then re-build afterward?

Yes, this is what I meant. So I might have misinterpreted your comment then, sorry.

I do like the analogy between fpm clean and the common make clean rule, in the sense that this command serves to return your project directory into it's initial state.

Agreed. A lot of people, I suspect, would be familiar with make clean.

[LKedward]

Thanks Andre @freevryheid. I agree that having a fpm clean command would be good however my preference would be that, by default, it only removes the build output files and not also the dependencies folder. (An additional flag could be specified to remove the dependencies also.)

Yes fpm build -d is also a possible option, however I caution against this because under normal operation it should not be necessary for users to frequently do clean+build. This would imply some deficiency with fpm (e.g. #463).

I like fpm clean as the command since that is what would be most expected

[certik]

Indeed, I often delete things by git clean -dfx, but this does not delete the dependencies, because those are typically git repositories and git doesn't remove them even with git clean -dfx. So I then have to manually remove. If we go with a new command fpm clean, then we can follow the same approach as git:

• fpm clean cleans but asks first, skip dependencies
• fpm clean -f cleans without asking, but skips dependencies
• fpm clean -df cleans without asking, including dependencies

If we add it to fpm build, then something similar can be done. I agree with @ivan-pi that adding a new command should not be taken lightly. Cargo has cargo clean. I think fpm clean -df would be used a lot, certainly by me.

Just yesterday I wrongly specified a branch of a dependency (master instead of a main), fpm failed and it kept failing even after removing the build directory. So fpm clean -df would fix it.

Overall, I think it's fine to add fpm clean.

[epagone]

Yes fpm build -d is also a possible option, however I caution against this because under normal operation it should not be necessary for users to frequently do clean+build. This would imply some deficiency with fpm (e.g. #463).

Good point and I agree. With this observation in mind, I'll take back my previous preference to provide also clean+build, clean+run, etc... fpm clean (with the proposed optional flags) should be enough.

[ivan-pi]

After reviewing cargo clean and all of your comments I concede that a separate sub-command, like suggested in this PR, is fine.

[LKedward]

Thanks for the discussion everyone, I think we've come to a decision on the specification for this as outline in Ondrej's comment. Andre @freevryheid, are you happy to update this PR accordingly?

[freevryheid]

Thanks, I'll give it a shot.

[wyphan]

I just added a CLI test for clean but can't directly push to the branch, so here it is as a .patch.txt file.
clean_cli_tests.patch.txt

[wyphan]

And here's another patch that adds the echo optional argument to the os_remove_dir function (default is .true., like in the mkdir function), as well as adding a new test for creating and deleting directories.
Edit: changed patch to use backslash when on Windows

test_mkdir_rmdir.patch.txt

[wyphan]

Btw, I tested on both bare-metal Linux (Ubuntu 20.04.4 LTS) and a Windows 10 VM (using @LKedward's quick start Fortran installer for Windows). I think this PR is ready to be merged, unless somebody wants to clean it up further to use OS_WINDOWS directly instead of os_is_unix().

[certik]

Thanks. Let's get this done. I am missing fpm clean quite a bit, have to do it manually each time.

[LKedward]

Many thanks Andre @freevryheid and Wileam @wyphan, and apologies for the delay in reviewing this.

@freevryheid are you able to take a look at the patches from @wyphan?

I hope to have some time to review and test this in the coming week.

[freevryheid]

wyphan's patches includes a fix for the issue discussed here: #668
i tried excluding this before staging the commit but messed up - it includes changes to the installer.f90 file.

[wyphan]

@freevryheid Huh that's weird, I didn't recall patching that file too (src/fpm/installer.f90). Rather than cleaning up the mess (I hate rebases and force pushes), can someone with the appropriate permissions link #668 here so this PR closes it once merged?

[freevryheid]

@freevryheid Huh that's weird, I didn't recall patching that file too (src/fpm/installer.f90). Rather than cleaning up the mess (I hate rebases and force pushes), can someone with the appropriate permissions link #668 here so this PR closes it once merged?

@wyphan, yep I made the changes to installer.f90 a while back - forgot they were in there when I merged and committed your patches.

[LKedward]

Many thanks again Andre @freevryheid and Wileam @wyphan, this all looks good to me 👍
And thanks for reviewing @awvwgk. If there aren't any further comments here, then I'll go ahead and merge this tomorrow.

[arjenmarkus]

Just a matter of style, but why the very terse options "-d" and "-D"? I would never be able to remember which one is which ;). Why not use complete words? Op di 15 mrt. 2022 om 13:26 schreef Emanuele Pagone < ***@***.***>:

…

Would it be too complicated to have both? I think those are two distinct, typical use cases: - you want to "clean" and then decide what to do (e.g. follow-up with fpm run or fpm test, etc...) - you know already you want to re-build or re-run and then you'll use the flags -d or -D — Reply to this email directly, view it on GitHub <#665 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAN6YR5QJ46A5FW5LY7A5UTVAB6WZANCNFSM5PRXMYKQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[epagone]

I think what was actually implemented is --all

[urbanjost]

NAME

clean(1) - delete the build

SYNOPSIS

fpm clean

DESCRIPTION

Prompts the user to confirm deletion of the build. If affirmative,
directories in the build/ directory are deleted, except dependencies.

OPTIONS

--skip delete the build without prompting but skip dependencies.
--all delete the build without prompting including dependencies.

USAGE:

KEYWORD   SHORT PRESENT VALUE
version      v  F        [F]
verbose      V  F        [F]
usage        u  T        [T]
skip            F        [F]
help         h  F        [F]
directory    C  F        [" "]
all             F        [F]

From the --usage key no short names were given for --skip and --all. It seems there are strong opinions about assigning short names to the commands in some discussions both for and against; but it would be a four-character change to the code to assign short names. Where it defines --all F you would change it to --all:a F, for example. Just add :LETTER after the name when it is defined in set_arg. Maybe there should be a discussion about whether short names should be required or eshewed.