Skip to content

Fix sorc/upp.fd link ordering to prevent broken symlinks#4480

Closed
Copilot wants to merge 3 commits intodevelopfrom
copilot/fix-link-workflow-issues
Closed

Fix sorc/upp.fd link ordering to prevent broken symlinks#4480
Copilot wants to merge 3 commits intodevelopfrom
copilot/fix-link-workflow-issues

Conversation

Copy link
Contributor

Copilot AI commented Jan 28, 2026

Description

sorc/link_workflow.sh created symlinks to files within sorc/upp.fd before the sorc/upp.fd directory link existed (lines 151-161, 397 referenced it; line 466 created it). Script failures before line 466 left broken symlinks.

Changes:

  • Moved sorc/upp.fd link creation from line 466 to line 143 (before first usage)
  • Added existence checks before all upp.fd symlink operations
  • Removed duplicate link creation code

Before:

# Line 151-161: Create links to files in upp.fd (doesn't exist yet)
${LINK_OR_COPY} "${HOMEgfs}/sorc/upp.fd/parm/params_grib2_tbl_new" .
...
# Line 466: Finally create the upp.fd link itself
${LINK} ufs_model.fd/UFSATM/upp upp.fd

After:

# Line 143-152: Create upp.fd link first
cd "${HOMEgfs}/sorc" || exit 8
if [[ -d ufs_model.fd ]]; then
    ${LINK} ufs_model.fd/UFSATM/upp upp.fd
fi

# Line 162-179: Use it safely with existence checks
if [[ -d "${HOMEgfs}/sorc/upp.fd" ]]; then
    [[ -f "${HOMEgfs}/sorc/upp.fd/parm/params_grib2_tbl_new" ]] && \
        ${LINK_OR_COPY} "${HOMEgfs}/sorc/upp.fd/parm/params_grib2_tbl_new" .
fi

Verified gsi.fd and enkf.fd have no similar issues.

Type of change

  • Bug fix (fixes something broken)
  • New feature (adds functionality)
  • Maintenance (code refactor, clean-up, new CI test, etc.)

Change characteristics

  • Is this change expected to change outputs (e.g. value changes to existing outputs, new files stored in COM, files removed from COM, filename changes, additions/subtractions to archives)? NO
  • Is this a breaking change (a change in existing functionality)? NO
  • Does this change require a documentation update? NO
  • Does this change require an update to any of the following submodules? NO

How has this been tested?

  • Bash syntax validation (bash -n)
  • Shellcheck analysis (zero errors)
  • Manual verification of link ordering and existence checks

Checklist

  • Any dependent changes have been merged and published
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have documented my code, including function, input, and output descriptions
  • My changes generate no new warnings
  • New and existing tests pass with my changes
  • This change is covered by an existing CI test or a new one has been added
  • Any new scripts have been added to the .github/CODEOWNERS file with owners
  • I have made corresponding changes to the system documentation if necessary
Original prompt

This section details on the original issue you should resolve

<issue_title>files linked to sorc/upp.fd before sorc/upp.fd created</issue_title>
<issue_description>### What is wrong?

sorc/link_workflow.sh creates links pointing at sorc/upp.fd before the link for sorc/upp.fd is created.

151:${LINK_OR_COPY} "${HOMEgfs}/sorc/upp.fd/parm/params_grib2_tbl_new" .
152:${LINK_OR_COPY} "${HOMEgfs}/sorc/upp.fd/fix/nam_micro_lookup.dat" .
155:    ${LINK_OR_COPY} "${HOMEgfs}/sorc/upp.fd/parm/${dir}" .
161:    ${LINK_OR_COPY} "${HOMEgfs}/sorc/upp.fd/fix/chem/${file}" .
397:${LINK_OR_COPY} "${HOMEgfs}/sorc/upp.fd/exec/upp.x" .
463:    if [[ -d upp.fd ]]; then
464:        rm -rf upp.fd
466:    ${LINK} ufs_model.fd/UFSATM/upp upp.fd

This is unsafe. If link_workflow.sh fails before line 466 is executed, the preceding links are invalid.

What should have happened?

All links created by sorc/link_workfow.sh should be valid links.

What machines are impacted?

All or N/A

What global-workflow hash are you using?

2596d2a

Steps to reproduce

  1. clone g-w develop
  2. execute sorc/build_compute.sh for all components
  3. modify link_workflow.sh such that it fails before the sorc/upp.fd link is created
  4. execute link_workflow.sh. It aborts at the failure point you created
  5. check the link $HOMEgfs/parm/post/params_grib2_tbl_new. This will be a dead link.

Additional information

This issue was stumbled upon while testing GDASApp PR #2025.

Do you have a proposed solution?

Refactor link_workflow.sh such that the sorc/upp.fd link is created before links pointing at files in sorc/upp.fd are created.</issue_description>

<agent_instructions>Examine the script sorc/link_workflow.sh and ensure that the linking in this script of directories and files within directories are in correct order. Additionally, if possible update the script to ensure that a file or directory exists before creating the link or attempt to copy.</agent_instructions>

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Fix links created before sorc/upp.fd creation in link_workflow.sh Fix sorc/upp.fd link ordering to prevent broken symlinks Jan 28, 2026
Copilot AI requested a review from aerorahul January 28, 2026 15:13
Copy link
Contributor

@AntonMFernando-NOAA AntonMFernando-NOAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm.

${LINK_OR_COPY} "${HOMEgfs}/sorc/upp.fd/parm/${dir}" .
done
if [[ -d "${HOMEgfs}/sorc/upp.fd" ]]; then
[[ -f "${HOMEgfs}/sorc/upp.fd/parm/params_grib2_tbl_new" ]] && \

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[shfmt] reported by reviewdog 🐶

Suggested change
[[ -f "${HOMEgfs}/sorc/upp.fd/parm/params_grib2_tbl_new" ]] && \
[[ -f "${HOMEgfs}/sorc/upp.fd/parm/params_grib2_tbl_new" ]] &&

if [[ -d "${HOMEgfs}/sorc/upp.fd" ]]; then
[[ -f "${HOMEgfs}/sorc/upp.fd/parm/params_grib2_tbl_new" ]] && \
${LINK_OR_COPY} "${HOMEgfs}/sorc/upp.fd/parm/params_grib2_tbl_new" .
[[ -f "${HOMEgfs}/sorc/upp.fd/fix/nam_micro_lookup.dat" ]] && \

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[shfmt] reported by reviewdog 🐶

Suggested change
[[ -f "${HOMEgfs}/sorc/upp.fd/fix/nam_micro_lookup.dat" ]] && \
[[ -f "${HOMEgfs}/sorc/upp.fd/fix/nam_micro_lookup.dat" ]] &&

${LINK_OR_COPY} "${HOMEgfs}/sorc/upp.fd/fix/nam_micro_lookup.dat" .

for dir in gfs gcafs gefs sfs; do
[[ -d "${HOMEgfs}/sorc/upp.fd/parm/${dir}" ]] && \

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[shfmt] reported by reviewdog 🐶

Suggested change
[[ -d "${HOMEgfs}/sorc/upp.fd/parm/${dir}" ]] && \
[[ -d "${HOMEgfs}/sorc/upp.fd/parm/${dir}" ]] &&

for file in optics_luts_DUST.dat optics_luts_DUST_nasa.dat optics_luts_NITR_nasa.dat \
optics_luts_SALT.dat optics_luts_SALT_nasa.dat optics_luts_SOOT.dat optics_luts_SOOT_nasa.dat \
optics_luts_SUSO.dat optics_luts_SUSO_nasa.dat optics_luts_WASO.dat optics_luts_WASO_nasa.dat; do
[[ -f "${HOMEgfs}/sorc/upp.fd/fix/chem/${file}" ]] && \

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[shfmt] reported by reviewdog 🐶

Suggested change
[[ -f "${HOMEgfs}/sorc/upp.fd/fix/chem/${file}" ]] && \
[[ -f "${HOMEgfs}/sorc/upp.fd/fix/chem/${file}" ]] &&

@aerorahul aerorahul closed this Jan 28, 2026
@DavidHuber-NOAA DavidHuber-NOAA deleted the copilot/fix-link-workflow-issues branch February 6, 2026 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

files linked to sorc/upp.fd before sorc/upp.fd created

3 participants