Skip to content
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

Add analogue to _concat for writable directories #4380

Closed
wants to merge 1 commit into from

Conversation

mwichmann
Copy link
Collaborator

The regular _concat function, when processing a list of strings, adds a prefix+suffix to each, possibly after expanding the list through a call to a function. The function is often RDirs, which can add additional paths (repositories, usually). This is used for things like path-to-include dir, so we get "-Iinclude" without storing the compiler-specific syntax.

When used for a directory which we tell the compiler to write to, as opposed to just search, this can give the wrong result. For example, for $FORTRANMODIR, which describes where to put generated module files, we only want to write to the target directory (and in fact, for gfortran, it's an error to specify the module-write-dir more than once), but we might want to still search the backing directories - any repository, and in case of non-duplicating variantdir, the directory of the original source - in case there were already module files there that can be used.

This new function wraps the first string from the expanded list in the prefix/suffix, and any remaining strings in the extra_prefix and extra_suffix arguments, for example to produce "-Jtargetmoddir -Isourcemoddir -Irepository" instead of "-Jtargetmoddir -Jsourcemoddir -Jrepository". The latter would abort the Fortran compiler with an error, and the equivalent for the D compilers would ignore the first (correct) directory and select the last one for use. In any case, we're not allowed to write to the repository, so the previous behavior using _concat was definitely wrong.

Contributor Checklist:

  • I have created a new test or updated the unit tests to cover the new/changed functionality.
  • I have updated CHANGES.txt (and read the README.rst)
  • I have updated the appropriate documentation

@mwichmann mwichmann added the Tools Issues related to tools subsystem label Jul 19, 2023
@bdbaddog
Copy link
Contributor

While this solution works, I'm not sure if specifying a list of directories of which only the first one is handled differently is the best solution?

If there's includepaths, and write to paths, shouldn't those be separate envvars?

@mwichmann
Copy link
Collaborator Author

Those two languages have cvars already for include paths, those are for ones the developer actually knows about - plus any possible backing copies in repositories. This is for the problem when SCons silently adds directories. The other alternative is to throw those extras away, perhaps by not calling RDirs - but you suggested earlier that wasn't a good idea, might want to search for already built modules in a repository.

The regular _concat function, when processing a list of strings, adds a
prefix+suffix to each, possibly after expanding the list through a call to
a function. The function is often RDirs, which can add additional paths
(repositories, usually). This is used for things like path-to-include
dir so we get "-Iinclude" etc.

When used for a directory which we tell the compiler to write to, as
opposed to just search, this can give the wrong result. For example,
for $FORTRANMODIR, which describes where to put generated module files,
we only want to write to the target directory (and in fact, for gfortran,
it's an error to specify the module-write-dir more than once), but we
might want to still search the backing directories - any repository,
and in case of non-duplicating variantdir, the directory of the original
source - in case there were already module files there that can be used.

This new function wraps the first string from the expanded list in
the prefix/suffix, and any remaining strings in the extra_prefix and
extra_suffix arguments, for example to produce

"-Jtargetmoddir -Isourcemoddir -Irepository"

instead of

"-Jtargetmoddir -Jsourcemoddir -Jrepository".

The latter would abort the Fortran compiler with an error, and the
equivalent for the D compilers would ignore the first (correct)
directory and select the last one for use. In any case, we're not
allowed to write to the repository, so the previous behavior using
_concat was definitely wrong.

Signed-off-by: Mats Wichmann <[email protected]>
@mwichmann mwichmann force-pushed the issue/fortran-and-d-dirs branch from 89125ac to c46ce1f Compare December 21, 2023 19:35
@mwichmann mwichmann closed this Jun 29, 2024
@mwichmann mwichmann deleted the issue/fortran-and-d-dirs branch June 29, 2024 16:50
@mwichmann
Copy link
Collaborator Author

I'll revisit this someday, no need to have it hanging around here for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Tools Issues related to tools subsystem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants