-
Notifications
You must be signed in to change notification settings - Fork 710
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 the ability for registry code generation to be out of source #1975
Conversation
@islas What happened to these files before this change when doing 'clean -a'? Will these files be removed after this change with 'clean -a' or else? |
The regression test results:
|
@weiwangncar This previously was done by hard-coding To roughly emulate this without changing too much, the io_boiler_plate_temporary.inc is generated at the top-level (from *io_boiler_plate_temporary.inc is currently not being deleted but if necessary I can add that to the clean script. The reason it was moved is slightly difficult to piece together due to how fragile the Registry code is but the reasoning is as follows:
|
@islas Thank you very much for the detailed explanation! Really appreciate it! |
It seems less than ideal to dump the @islas What do you think about the idea of adding a new optional argument to If the |
If we were to go the route proposed in my previous comment, I suppose there would be two directory arguments to |
Do you mean generalizing search to an arbitrary number of search paths? While I think we could, I question whether it would be worth it at this stage in the registry's life where expansion is generally minimal.
I think I had considered similar options, and my original reason was avoiding manipulation of the interfacing while still being more or less reasonable. Having both the CMake and Make builds fundamentally do the same thing (writing to I do think having the |
I think "generalization" could be as restricted as allowing the alternate directory (i.e., non- To start some further discussion, where are other files that are generated by the registry being written in out-of-source builds using CMake? I assume this is an |
Structurally different, but in binary execution they are 1-to-1 the same so I wouldn't necessarily say fundamentally different. I think this is a worthwhile aspect, as the concept of introducing various modes of operation to achieve the same effect that a single mode could can lead to overly complex systems. What we could do - if leaving the boiler plate at the top-level is the primary concern - is change the secondary search path to instead be
This second search is still needed because registry generation of files is all relative, but the search paths are whatever comes before the Registry file. For CMake this is an absolute path and a different path than the boiler plate which is now in Benefits:
Other files are written out in the same relative locations as before, |
I think that sounds reasonable. The placement of generated code in the top-level If I'm understanding correctly, then, there will be effective no modifications to |
Yes, that will be the end result. I'll go ahead and push the changes after checking that it does in fact do what we want for both builds just to be sure. |
The last change passed the regression test. |
@islas The new code diff looks good to me. Can you take another look at the PR description to see if we can more clearly explain the purpose and motivation for the changes? A few points that would be worth capturing include:
(Note: I may not have gotten the above details correct, which is all the more reason to document them thoroughly in the PR description.) |
I updated the PR description previously, which may have gotten some of those points already, but aside from point 3, I think the further additional edits I just made should capture the rest. Point 3 is not correct as the search path is whatever is before the Registry file provided on the command line, which could be relative or absolute. |
Thanks for the edits. I think the "Problem" description (i.e., "The registry generated code clutters the source development environment with hundreds of files, requiring many .gitignore entries that seem counterintuitive (e,g. ignoring all *.f90 source files)") still seems unrelated to any of the actual changes in the PR. Could you take another pass at that? |
@mgduda Should flow a bit better now |
Thanks! |
TYPE: enhancement
KEYWORDS: registry, code generation
SOURCE: internal
DESCRIPTION OF CHANGES:
Problem:
The current registry generated code assumes generating the autogenerated code should be placed within the WRF repository alongside true source code that is committed to the repository. This leads to a cluttered source tree with hundreds of files that were autogenerated, and relies on various stopgaps (unintuitive ignores, specific path and wildcard
rm
commands within source folders, run code generation from specific location).In preparation for the future cmake build, it would be ideal to not inherit these limitations of the registry code registration, allowing the source tree to be pristine while autogenerated code is placed in an alternate build directory.
Solution:
Add the potential for out-of-source code generation of the registry code. This can be achieved by adding the ability search a relative directory for the
io_boiler_plate_temporary.inc
. This would allow the registry generation to occur in an alternate directory but still source the Regsitry/ config files. This is because the boiler plate is generated relative to the execution path, but Registry config files are sourced via the provided command line path.Without this change, while the makefile system would run as before registry code generation outside the WRF directory structure would place the boiler plate and Registry config files in two different locations and thus fail to find the core template file. Since the relative directory of the boiler plate output is
./Registry
, by searching for that as the alternate relative directory we would find both Registry config files and the generated boiler plate. This would be able to accommodate different locations while for the makefile build system there would be no change to the way code is generated and searched as./Registry
and path to the Registry config files are the same when at the top-level WRF directory.LIST OF MODIFIED FILES:
M tools/reg_parse.c
TESTS CONDUCTED:
RELEASE NOTE:
Add the ability for registry code generation to be out of source