-
Notifications
You must be signed in to change notification settings - Fork 114
Add more example packages #178
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
👍 from me. Do you agree on also adding this example package:
where module greet_m
implicit none
character(*), parameter :: greeting = 'Hello, fpm!'
end module greet_m
program module_in_program_source
use greet_m, only: greeting
implicit none
print *, greeting
end program module_in_program_source We're supporting this implicitly without special care, but we should make sure to not break the support. |
Thanks @milancurcic. Good point yes I agree, I'll add that example in too. |
I'm not sure I agree that we do want to support having a module in the same file with a program. We don't support having multiple modules in one file. Is there a reason for wanting this capability? |
We do, I just tried it. :) I get that many people may not care about this, but I think you need a strong argument to disallow valid code. |
I similarly find it useful now and again to put a module and a program together for small supporting programs like tests, demos and benches. This is fairly common usage I believe. My main quarrel with disallowing this, and other valid fortran layouts, is with the enforcement of design decisions on users by constraint. This is not the job of a build tool IMO. Regarding existing support, I believe it is supported to the extent that the module in question is not used by any other file. |
I find it somewhat useful when tooling (if not the language itself) can at least discourage known, poor design decisions. Similar to the way most modern languages don't have a Are there any other languages that allow multiple modules in one file? |
If a program is found at the end of a source file, it will override any previous definition of unit_type to FPM_UNIT_PROGRAM.
If a module dependency is satisfied in the same file, it is not added to file_dependencies. Use two-pass procedure to count the number of actual file_dependencies required for module resolution.
I agree with Brad. But similarly I am for restricting the naming of modules to enforce directory structure, but if I recall correctly, Brad was against.
The fact that current codes use something is not a good argument because current codes would require some modifications anyway to compile with fpm.
I think a good way forward is to restrict with an fpm.toml option (we can discuss whether the option should be on or off by default). That way we can satisfy both camps.
I definitely would like an option in fpm to warn against non standard usage.
Huge advantage of being strict is that it simplifies the available options, which makes it easier to understand other people's codebases.
…On Fri, Sep 18, 2020, at 4:28 PM, Brad Richardson wrote:
I find it somewhat useful when tooling (if not the language itself) can
at least discourage known, poor design decisions. Similar to the way
most modern languages don't have a `goto`. But if there is sufficient
desire, and some use cases where it would lead to better design, I'm ok
with it. I don't know if that's the case, but I'm open to being shown
some.
Are there any other languages that allow multiple modules in one file?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#178 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAFAWHVXFLWYQYOSEHW5DLSGPGI3ANCNFSM4RR2MP5A>.
|
Regarding quick tests: "fpm new" will create you an executable with a module, properly in separate files, so I intend to use that.
…On Sat, Sep 19, 2020, at 11:37 AM, Ondřej Čertík wrote:
I agree with Brad. But similarly I am for restricting the naming of
modules to enforce directory structure, but if I recall correctly, Brad
was against.
The fact that current codes use something is not a good argument
because current codes would require some modifications anyway to
compile with fpm.
I think a good way forward is to restrict with an fpm.toml option (we
can discuss whether the option should be on or off by default). That
way we can satisfy both camps.
I definitely would like an option in fpm to warn against non standard usage.
Huge advantage of being strict is that it simplifies the available
options, which makes it easier to understand other people's codebases.
On Fri, Sep 18, 2020, at 4:28 PM, Brad Richardson wrote:
>
>
> I find it somewhat useful when tooling (if not the language itself) can
> at least discourage known, poor design decisions. Similar to the way
> most modern languages don't have a `goto`. But if there is sufficient
> desire, and some use cases where it would lead to better design, I'm ok
> with it. I don't know if that's the case, but I'm open to being shown
> some.
>
> Are there any other languages that allow multiple modules in one file?
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> <#178 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAAFAWHVXFLWYQYOSEHW5DLSGPGI3ANCNFSM4RR2MP5A>.
>
|
I agree with having fpm print a warning (recommendation, really) if it finds multiple modules or module + program in a source file. I don't like the idea of restricting this with an option because if you expressly don't want to do this, then you're already not writing code that needs to be restricted. Conversely, if the default is to restrict it, then having to add an option to enable it somewhat defeats the purpose of me using it for convenience. If you're against a correct default behavior that is supported in the present and want to disable it as a feature, you need to convince others, not the other way around. "known, poor design decisions" is subjective and not meaningful without elaborating why. |
Technically the main issue as I see it is that if we start with being restrictive, we can always relax the requirements later without breaking any existing fpm package. However, if we relax the requirements now, we can't easily make them more strict without breaking existing packages.
Regarding the warning, I don't know if I would like to be getting warnings for things that are allowed. In other words, Milan, would you like to be getting warnings for modules in the same file as the main program?
Summary: let's be very conservative with relaxing restrictions. It's a one way street.
…On Sat, Sep 19, 2020, at 1:04 PM, Milan Curcic wrote:
I agree with having fpm print a warning (recommendation, really) if it
finds multiple modules or module + program in a source file.
I don't like the idea of restricting this with an option because if you
expressly don't want to do this, then you're already not writing code
that needs to be restricted. Conversely, if the default is to restrict
it, then having to add an option to enable it somewhat defeats the
purpose of me using it for convenience.
If you're against a correct default behavior that is supported in the
present and want to disable it as a feature, you need to convince
others, not the other way around. "known, poor design decisions" is
subjective and not meaningful without elaborating why.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#178 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAFAWHVGF2O6K2SQWD4DPLSGTXDZANCNFSM4RR2MP5A>.
|
I don't think we should consider deferring this decision. I think Milan makes a very important point: having no restrictions does not affect those who want to conform to their own standard for layout, but enforcing restrictions does affect those who do not. By placing restrictions on otherwise valid Fortran you are enforcing one group's subjective preference on everyone.
There are some situations where I learn to live with certain compiler warnings, because I know why those warnings exist and why I have chosen to ignore them. This would be the same in this case. |
Note that this argument when followed to its logical conclusion would prevent us to impose any kind of "default layout" in fpm, because it is preventing valid code to compile.
Are we allowing multiple modules or just one module together with the program? If multiple, it means we are also making a decision of not ever imposing the module naming convention based on the filename?
There are huge advantages that come from having a restrictive default layout. I want to make sure all the implication of this decision are understood.
If we make this decision, will it be warning by default, and you can turn off the warning in fpm.toml with an option? That actually would be fine with me.
…On Sun, Sep 20, 2020, at 2:52 AM, Laurence Kedward wrote:
> Technically the main issue as I see it is that if we start with being restrictive, we can always relax the requirements later without breaking any existing fpm package.
I don't think we should consider deferring this decision.
I think Milan makes a very important point: having no restrictions does
not affect those who want to conform to their own standard for layout,
but enforcing restrictions does affect those who do not. By placing
restrictions on otherwise valid Fortran you are enforcing one group's
subjective preference on everyone.
> Regarding the warning, I don't know if I would like to be getting warnings for things that are allowed.
There are some situations where I learn to live with certain compiler
warnings, because I know why those warnings exist and why I have chosen
to ignore them. This would be the same in this case.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#178 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAFAWEDC6BPNNUGYSS7II3SGW7ERANCNFSM4RR2MP5A>.
|
Yes, a warning by default.
Are these advantages for fpm developers or advantages for fpm users? My argument is based on prioritising the latter over the former. |
I am fine with warning by default.
I think the advantages are for the users. I agree that is what we should prioritize.
We can discuss further at our monthly call. I think we have an agreement on this particular issue to move forward.
…On Sun, Sep 20, 2020, at 7:14 AM, Laurence Kedward wrote:
> If we make this decision, will it be warning by default, and you can turn off the warning in fpm.toml with an option?
Yes, a warning by default.
> There are huge advantages that come from having a restrictive default layout.
Are these advantages for fpm developers or advantages for fpm users? My
argument is based on prioritising the latter over the former.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#178 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAFAWGQ4B6K6KMAWOUBM6DSGX543ANCNFSM4RR2MP5A>.
|
Thanks @certik. Perhaps a nuanced but important point that may have been missed: I don't argue here for relaxing any restriction. I argue for including a test case of a pattern that is presently both permitted (within our planned restriction set) and working, rather than leaving it untested. I was assuming that the file/module layout and naming restriction we impose follows from #153 where we agreed that we will require the module name to be prefixed with package name. This helps prevent name collisions. I was also assuming that we don't anymore require the module name to be the same as the source file name. I don't think it does anything for us given #153. Do you agree? So, if we don't require the module name to be the same as the source file name, I see no practical reason to forbid module+program or multi-module in source file. However, if we do require it, then we can't do what I proposed in this thread. As an afterthought, given #153 we should rename the modules in current example packages to reflect this (even though we're not enforcing it yet). |
My understanding was that we relaxed the name of the modules, but kept the door open if we wanted to make it strict again. With this change the door closes.
I think the overall goal is for fpm to fully understand the project and to build it automatically.
I think this change doesn't compromise this goal, so I think it's fine to relax restrictions.
The other goal is to make it easy to use a dependency. For that if we will not enforce module names based on directories, we could make a command "fpm api PACKAGE" that would summarize what modules the user can call and what is in them.
…On Sun, Sep 20, 2020, at 9:33 AM, Milan Curcic wrote:
Thanks @certik <https://github.com/certik>.
Perhaps a nuanced but important point that may have been missed: I
don't argue here for relaxing any restriction. I argue for including a
test case of a pattern that is presently both permitted (within our
planned restriction set) and working, rather than leaving it untested.
I was assuming that the file/module layout and naming restriction we
impose follows from #153
<#153> where we agreed that
we will require the module name to be prefixed with package name. This
helps prevent name collisions.
I was also assuming that we don't anymore require the module name to be
the same as the source file name. I don't think it does anything for us
given #153 <#153>. Do you
agree?
So, if we don't require the module name to be the same as the source
file name, I see no practical reason to forbid module+program or
multi-module in source file. However, if we do require it, then we
can't do what I proposed in this thread.
As an afterthought, given #153
<#153> we should rename the
modules in current example packages to reflect this (even though we're
not enforcing it yet).
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#178 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAFAWGROTVUNT2WSW4USTLSGYODJANCNFSM4RR2MP5A>.
|
I think all doors should stay open. If we later decide to make the module naming more strict, we remove this test. If it shows to be problematic for implementation, we remove it. Adding this test doesn't close any doors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's merge this. We can improve upon this later.
Thanks @LKedward !
modules_provided
array;