fpm icon indicating copy to clipboard operation
fpm copied to clipboard

Enforce naming convention for modules

Open certik opened this issue 1 year ago • 12 comments

I am aware that we relaxed the strict naming convention enforcement of nested subdirectories a/b/c/a.f90 to a name of the module package_a_b_c_a, however the agreement was to keep enforcing the package_* prefix, where package is the name of the package.

Based on this comment, "At present, fpm is not enforcing this".

So we should change it and start enforcing it again, and create an optional "opt out" if you do not want this enforcement for your (legacy) package. It should also be encouraged for all new projects (and enforced by default).

certik avatar Aug 05 '22 09:08 certik

How should the prefix be handled for packages with a dash in the name?

awvwgk avatar Aug 05 '22 11:08 awvwgk

I’m of the opinion we should enforce this only on the “publishing” side, once we finally have an official registry/repository. I wouldn’t want a new user to get confused about this requirement.

everythingfunctional avatar Aug 05 '22 12:08 everythingfunctional

How should the prefix be handled for packages with a dash in the name?

Replace dashes with underscores, just like fpm new does.

everythingfunctional avatar Aug 05 '22 12:08 everythingfunctional

naming convention enforcement of nested subdirectories a/b/c/a.f90 to a name of the module package_a_b_c_a

Where would I put the module named package? This convention would suggest that src/package.f90 should contain a module named package_package. Then convention I've been using in my packages has been

src
├── package
│   ├── mod1.f90
│   ├── mod2.f90
│   └── ....f90
└── package.f90

everythingfunctional avatar Aug 05 '22 12:08 everythingfunctional

Replace dashes with underscores, just like fpm new does.

You could also remove them toml-f -> tomlf

awvwgk avatar Aug 05 '22 12:08 awvwgk

Where would I put the module named package?

If I understand correctly, and to clarify for everyone else, the suggestion here is NOT to enforce module naming based on directory structure. Rather, this issue is to enforce prefixing of modules with the package name only.

It makes sense to have a special case for modules named package which need not be redundantly prefixed with package.

LKedward avatar Aug 05 '22 12:08 LKedward

New user to Fortran would follow the tutorial and fpm generated new project, so there would be no problem. For existing projects you would add an option to fpm.toml, and you would need some tailoring anyway. A nice error message saying “Rename a to mypackage_a” and pointing at the file is not confusing.

Yes, this issue is to simply enforce prefixing all modules with the package name. If we all agree that this is good practice, then our tools should enforce it, and this "issue" (of conflicting modules) is forever fixed without a hassle. Sort of like enforcing automatic formatting by the CI, it just completely eliminates debates about formatting.

certik avatar Aug 05 '22 16:08 certik

New user to Fortran would follow the tutorial and fpm generated new project, so there would be no problem. For existing projects you would add an option to fpm.toml, and you would need some tailoring anyway. A nice error message saying “Rename a to mypackage_a” and pointing at the file is not confusing.

An implementation that seemed helpful enough could convince me.

Yes, this issue is to simply enforce prefixing all modules with the package name. If we all agree that this is good practice, then our tools should enforce it, and this "issue" (of conflicting modules) is forever fixed without a hassle. Sort of like enforcing automatic formatting by the CI, it just completely eliminates debates about formatting.

This does seem like a pretty huge win for, and makes it worth trying.

everythingfunctional avatar Aug 05 '22 16:08 everythingfunctional

An implementation that seemed helpful enough could convince me.

Brad, you should be more excited about such features. :) I was hoping you could implement it!

certik avatar Aug 05 '22 18:08 certik

I was hoping you could implement it!

I will have to see when I can fit it in. Of course anybody who might be interested shouldn't wait on me.

everythingfunctional avatar Aug 05 '22 18:08 everythingfunctional

I'm going to take the lead on this one, please assign me to this task if you wish. Here's a breakdown of what I think is most needed to achieve "module namespacing", at least the version discussed so far (fpm-project-name + module-name) without breaking current fpm behavior.

The initial work should focus on enabling this functionality I think.

  • [ ] Fortran modules are currently handled as strings. Replace them with a type(module_t) which extends a string;
  • [ ] Add functionality to the module that returns a "namespaced" name where applicable, or falls back to the simple name where not; I suggest the initial "namespaced" naming convention to be packagename_FPM_modulename. That's in line with how GCC internally names modules, and can be extended upstream to versions.
  • [ ] Add tests that verify edge cases, with and without namespaced .mod files.

I think the namespaced name better apply closest to the module name so to uniquely identify it, also when dealing with folders. We want .mod files be named the same way as the namespaced module, like package_FPM_module.mod, but let's discuss!

When this works robustly enough, namespacing should be extended through fpm to enable version checkout from the registry. The safest way I see it is:

  • [ ] type(module_t) should become a full type (not an extended string anymore), and contain an optional reference to its owner package (package_t).
  • [ ] Package definition and module uniquely identify a namespaced module name, to be enforced through the build process.
  • [ ] Now enable user to specify otherwise colliding module namespaces through fpm.toml, with a specific keyword (package? source? namespace?)

@minhqdao @arteevraina @henilp105

perazz avatar Dec 27 '22 09:12 perazz

Checkout https://github.com/fortran-lang/fpm/pull/828 and https://github.com/fortran-lang/fpm-docs/pull/90: base implementation completed IMHO.

perazz avatar Jan 20 '23 08:01 perazz