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

Wrong argument order in cdeps buildlib #104

Closed
billsacks opened this issue Jul 10, 2021 · 5 comments · Fixed by #136
Closed

Wrong argument order in cdeps buildlib #104

billsacks opened this issue Jul 10, 2021 · 5 comments · Fixed by #136
Assignees
Labels
bug Something isn't working

Comments

@billsacks
Copy link
Member

I think the argument order on this line is wrong:

caseroot, libroot, bldroot = parse_input(args)

As noted in ESMCI/cime#4034, this is using argument parsing that is appropriate for components, which (confusingly!) differs from the order of arguments in calls to libraries' buildlibs. CDEPS is built using the library code, not the component code.

See (for example) buildlib.csm_share for what I think is a correct argument parsing.

This is only relevant if CDEPS's buildlib is called as a subprocess instead of as a function. Typically it is called as a function, so this isn't critical, but it would be good to fix it to avoid subtle issues in the future.

@jedwards4b @mvertens @uturuncoglu

@billsacks billsacks added the bug Something isn't working label Jul 10, 2021
@jedwards4b jedwards4b self-assigned this Jul 29, 2021
@mvertens
Copy link
Collaborator

@jedwards4b - can you please look at this. Has this been resolved?

@jedwards4b
Copy link
Contributor

@jedwards4b
Copy link
Contributor

Note that the buildlib.csm_share is using it's own copy of parse_command_line and not the one in CIME/buildlib.py

@billsacks
Copy link
Member Author

@jedwards4b - The (confusing) situation is that the argument order differs when CIME calls component buildlib scripts vs. library buildlib scripts. Typically the library buildlib scripts are called as python functions rather than subprocesses, so it doesn't really matter, but if they were called as a subprocess, it would matter. It looks like https://github.com/ESMCI/cime/blob/master/scripts/lib/CIME/buildlib.py is intended for component buildlibs, not for library buildlibs. So library buildlibs need their own parse_command_line, and cannot use the version in CIME's shared buildlib.py.

There are some additional details in ESMCI/cime#4034

@billsacks
Copy link
Member Author

I thought I'd try to wrap this one up this afternoon, since I thought I knew what was needed. It turns out it's a little trickier than I first realized because the same buildlib script is used for both (1) building the CDEPS shared library, and (2) building the individual data models. The problem is that the argument order differs in those two cases, so fixing one breaks the other.

It looks like there is essentially no code shared between these two situations, though, so my plan is to introduce a new buildlib_comps that is used for building the individual data models, then trim each one down to only contain what's actually needed.

billsacks added a commit to billsacks/CDEPS that referenced this issue Dec 4, 2021
The argument order differs for the cdeps library vs. the individual
components. Thus, we need a different function to parse the command line
for the library. The new parse_command_line function is a direct copy of
the same function from buildlib.csm_share.

To handle this difference in command-line arguments in the two
situations, I have renamed the old file to buildlib_comps (which is
identical to the old buildlib); the individual data model components now
point to that file, leaving buildlib just being used for the cdeps
shared library.

In a following commit I will remove unnecessary code from each of these
files.

Resolves ESCOMP#104
Repository owner moved this from In Progress to Done in CESM: infrastructure / cross-component SE priorities Dec 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done (or no longer holding things up)
Development

Successfully merging a pull request may close this issue.

3 participants