should `checkModule` in `masonPublish` only count files?

[lucaferranti]

Currently checkModule does

/* Checks to make sure the package only has one main module
 */
private proc moduleCheck(projectHome : string) throws {
  const subModules = listDir(projectHome + '/src');
  if subModules.size > 1 then return false;
  else return true;
}

hence listDir will count both files and directories, hence if my file has a structure

M.chpl
M/
    L.chpl

subModules will be 2, leading the test to fail, despite it has only one submodule and one main module. Currently the function checks that the src folder has only one child. Currently I think that test will only pass if src has exactly one file and no subdirectories.

Moreover, I am a bit puzzled by the soundness of the check, the idea of the function is to check that the function has only one main module. Shouldn't the correct logic be something like

private proc moduleCheck(projectHome : string) throws {
  const mainModules = listDir(projectHome + '/src', dirs=false); // count only files, ideally filter for only .chpl ones
  if mainModules.size == 1 then return true;
  else return false;
}

This would rely on the following assumptions

1. files in src are considered main modules
2. files in subfolders are submodules

which I believe is more in line with what described here and here.

on the other hand the documentation also says

For packages that span multiple files, the main module is designated by the module that shares the name with the package directory and the name field in the Mason.toml file.

so one could also use this logic in the check, although not sure if the compiler is aware of this.

[benharsh]

I think you're absolutely right about the listDir being wrong. We'll take a look.

[lucaferranti]

Thanks @benharsh for the reply! I think the fix itself (updating moduleCheck) will be relatively straightforward. I would like to understand the motivation for that check in the first place.

Suppose I have a library MyPkg. Here are three options for src/

Option 1

src/
   MyPkg.chpl

This will be fine, src has only one file, hence only one module, hence only one main module. However, this solution makes sense only for small and simple libraries.

Option 2

src/
   MyPkg.chpl
   something.chpl
   somethignElse.chpl

Do something.chpl and somethingElse.chpl count as submodules of MyPkg.chpl or are they peer main modules? If I read

or packages that span multiple files, the main module is designated by the module that shares the name with the package directory and the name field in the Mason.toml file.

I would understand from it that this structure should still be fine, since MyPkg.chpl is the only main module. However maybe this is a mason specific convention and can causes issues, so this could be forbidden I guess.

Option 3

src/
   MyPkg.chpl
   MyPkg/
       something.chpl
       somethingElse.chpl

This structure will currently fail (because of my diagnosis above) but based on my reading of the docs I think it should be accepted. Is there any problem with this? I tried a small toy example and mason build and mason test with no additional flags seemed to work for this.

[jeremiah-corrado]

I created a draft PR to fix this; however, it does allow Option 2 above, and I'm wondering whether that should be legal.

I suspect that the second case could be a problem if MyPkg.chpl has a public use for the modules something and/or somethingElse because there could be module-resolution conflicts if someone tries to use MyPkg in their project that already has a module with the same name. I'll investigate whether this is an issue before going further with my PR.

Or if anyone happens to already know that Option 2 should be illegal that would be helpful.

[jeremiah-corrado]

Looking at this some more, I think we should make Option 2 illegal, to avoid potential module-resolution ambiguities. This requirement can always be relaxed in the future if we decide it's too strict.

[jeremiah-corrado]

I think this issue can be closed due to https://github.com/chapel-lang/chapel/pull/25084 and https://github.com/chapel-lang/chapel/pull/25098