Enforce module naming

[perazz]

Draft PR to implement enforced module naming conventions, see #823

[perazz]

The base implementation is completed, but I would like to discuss with the team @fortran-lang/fpm @everythingfunctional @milancurcic @certik @arteevraina @henilp105 @minhqdao before moving forward with tests and other:

• The naming convention is that each module should begin with the fortrannized (- becomes _) package name

• Like agreed in Add support for namespaced modules #823, the package-based naming convention is enforced by default, and one can opt out by running --no-module-names (see 194f051)

Output is decently informative, but mismatches are treated as fatal errors. For example in the hello_fpm example, we have

"New" enforced names:

federico@Federicos-MBP hello_fpm % fpmx build 
 ERROR: Module greet_m in ../hello_complex/source/greet_m.f90 does not match its package name (hello_complex).
 ERROR: Module farewell_m in ../hello_complex/source/farewell_m.f90 does not match its package name (hello_complex).
 ERROR: Module subdir_constants in ../hello_complex/source/subdir/constants.f90 does not match its package name (hello_complex).
        Hint: Try disabling name enforcing with --no-module-names . 
<ERROR>*cmd_build*:model error:The package contains invalid module names. Naming conventions are being requested.
STOP 1

Revert to old behavior:

federico@Federicos-MBP hello_fpm % fpmx build --no-module-names
constants.f90                          done.
greet_m.f90                            done.
farewell_m.f90                         done.
libhello_fpm.a                         done.
main.f90                               done.
hello_fpm                              done.
[100%] Project compiled successfully.

Since this change will break most existing projects including most of the fpm examples, another option would be to allow people some time to change their packages:

• do not enforce names by default, for now
• prompt the user that it will happen soon, in a future release (e.g. when the registry goes live)

[perazz]

Another thought: the current prefixing rule does not need a separator like _. I think that could lead to name collisions down the road. For example:

Package date could contain module datetime (prefix rule OK).
Package datetime could contain module datetime too, causing a module collision if both are used in the same project.

If a separator was mandatory, we would instead have module date_time from package date not colliding with module datetime from package datetime.

So in hindsight it would seem that using a separator could spare us from some of these edge cases.

[everythingfunctional]

Another thought: the current prefixing rule does not need a separator like _. I think that could lead to name collisions down the road. For example:

Package date could contain module datetime (prefix rule OK). Package datetime could contain module datetime too, causing a module collision if both are used in the same project.

If a separator was mandatory, we would instead have module date_time from package date not colliding with module datetime from package datetime.

So in hindsight it would seem that using a separator could spare us from some of these edge cases.

That is actually a good point. I hadn't considered that. Note that we haven't prevented a package named date_time, so you could still end up with collisions. Perhaps we need to think harder about it?

• opt out by running --no-module-names

I think it should be an option in the fpm.toml file. It will get old real fast having to type that argument all the time. We can then prevent publishing packages to the registry that have this option set.

[perazz]

Note that we haven't prevented a package named date_time, so you could still end up with collisions. Perhaps we need to think harder about it?

Yes - I think that adding the separator would exclude further potential issues, but the obvious bottleneck is Fortran requiring unique names <=63 chars; begin with a letter; case insensitive; only allowed symbol is _.

That is far more restrictive than what name a package could have. However, if we think in terms of using registries, which have namespaces to resolve further complexity, the odds of having a project require dependencies with name collisions will be very low, which the package maintainers can still cope with by slightly changing their package names.

[perazz]

A cool F77-like option would be to hash the package name to a unique 6-character identifier, but I don't think anyone would be happy to rename their packages with some random 6-character string.

[perazz]

I think it should be an option in the fpm.toml file.

This is a very good idea. Remove CLI option? Or, CLI overrides fpm.toml overrides default?

[perazz]

@everythingfunctional if you agree I'll open a separate PR to start documenting the basic Fortran naming rules and the package naming conventions in the fpm documentation on the basis of the PyPI example.

On the separator, I agree my original _FPM_ idea was really ugly. How about just a double underscore __ instead? Its uncommon enough that it would be a decently unique package/module separator I think. So we would prescribe

• package names are case insensitive and match Fortran naming conventions
• package names cannot have double underscores, to avoid confusion identifying the separator
• All module names in registry packages begin either with the package name, or with the package__module pattern.

The datetime case would resolve to:

use date__time ! module from package date
use datetime   ! from package datetime
use date_time  ! from package date_time

[everythingfunctional]

That sounds reasonable. Go ahead.

[perazz]

Ready for merge IMHO @everythingfunctional @certik @henilp105 @arteevraina @minhqdao

[certik]

I think that looks good. I have not checked the actual checking logic if it does what we need, but I assume that we can iterate on if it is not correct.

[perazz]

Thank you - I think the only big decision right now is about the package__module separator. It seems like two underscores are less elegant but also less confusing than one, as the single _ is commonly used in names, so enforcing that the package name cannot have it would be too restrictive IMHO. Otherwise, we've come up with a good solution I think

[certik]

Why not enforcing the package name with one prefix by default. Then allow overriding the prefix to enforce in fpm.toml?

[everythingfunctional]

I think this a reasonable design and looks reasonable to me.

[perazz]

I think I've finished the flexible prefix implementation. See more details at the docs page. Now we have two options, a more flexible and a stricter one, I think these will save us 99.9% of the headaches.

This PR at toml-f shows how simple it is to enforce naming with the customized prefix rule. It paves the way to enforcing these rules by default building fpm: all custom prefixes of the dependencies are loaded from their fpm.toml and enforced during build.

The latest commit also paves the way to build fpm itself with module naming enforced: I've renamed all module test* to module fpm_test*, so when we're ready we can just set module-naming="fpm" in fpm.toml.

When we're all on the same page with these updates, we can start enforcing naming here and there. Note that the registry will have to keep a list of custom prefixes, so when a user uploads a new package / wants to change it, they're granted or forbidden permission to do it.

[everythingfunctional]

Hi @perazz, I haven't had a chance to look at the implementation, but the description of the rules (here) sounds pretty good. However,

I've renamed all module test* to module fpm_test*

This should not be necessary. The prefix rules should only be enforced for library modules (i.e. src). app and test files are not used for dependencies, so no worry about naming conflicts there.

[awvwgk]

Following up on toml-f/toml-f#121, I would suggest that we allow a transition phase of one release in fpm for packages to adopt the new change and also to avoid breaking the build of fpm itself.

1. add build.module-naming entry, if present it is enforced by fpm
2. release new fpm version
3. add build.module-naming in fpm dependencies
4. make complying with module naming mandatory in fpm
5. release new fpm version

This way we can avoid deadlocking us due to TOML Fortran containing a dash in the package name.

[perazz]

The current PR does not enforce names by default so it fits item 1. well imho.

[awvwgk]

Looks good to me. With three approvals I will go ahead and merge.