GH workflow builds against ngen submodule commit, not current commit

[hellkite500]

noah-owp-modular/.github/workflows/ngen_integration.yaml

Line 95 in 05aec15

- name: Build Noah-owp-modular Surfacebmi

Just noticed that the workflow here is using the ngen build-submodule action to build the NOM library to test the integration with. This will cause the runner to use the pinned ngen submodule commit to build form, not the current code. This likely isn't the intended behavior here.

Can either toy with the ngen submod build action to try to get the correct code/pr-branch updated in the runner's ngen clone, or simply build the ngen lib from source (would be easier once #103 is complete). Could also hack somewhere in the middle and use the checked out code to overwrite the dir and try to re-build via a second action step.

Either way, something to think about for getting a local integration test setup.

[SnowHydrology]

@hellkite500, other ngen-build workflows are failing (e.g., in evapotranspiration). Do you think this is the cause across formulations?

[madMatchstick]

@hellkite500, other ngen-build workflows are failing (e.g., in evapotranspiration). Do you think this is the cause across formulations?

So, there's kind of two unique-ish things going on here,

1. This issue Nels just outline. Most (if not all?) ngen_integration.yml workflows (across all implemented formation repos) directly call an existing 'ngen' action to build (sub)module libraries. Although convenient because the set-up already exists, this is not exactly what we want because action pulls whatever pinned commit is currently in ngen/extern from the submodules master/main branch. So we aren't even running the integration workflow with updated code from PR. At glance, I'm thinking ${{ github.head_ref }} maybe worth investigating...
2. Because the NOM build requires a -DNGEN_IS_MAIN_PROJECT flag, the use of the linked action in (1) to build NOM specifically, now breaks as ngen issue #761 addresses. See proposed ngen pr #768 if this helps clarify.

@hellkite500 can correct me if I'm off. I believe getting item (1), this issue #104, is more priority.

[stcui007]

It looked like this line of codes in ngen-submodule-build/action.yaml: run: git submodule update --init --recursive -- ${{ inputs.mod-dir }} over wrote the newest version we checked out earlier and moved back to extern/. We should build the submodule before checkout ngen and move the already built sub-module to extern, not build using ngen submodule action.

…

On Tue, Mar 19, 2024 at 1:29 PM JessicaGarrett-NOAA < ***@***.***> wrote: @hellkite500 <https://github.com/hellkite500>, other ngen-build workflows are failing (e.g., in evapotranspiration <https://github.com/NOAA-OWP/evapotranspiration/actions/runs/8299846846/job/22716483404?pr=31>). Do you think this is the cause across formulations? So, there's kind of two unique-ish things going on here, 1. This issue Nels just outline. Most (if not all?) ngen_integration.yml workflows (across all implemented formation repos) directly call an existing 'ngen' action <https://github.com/NOAA-OWP/ngen/blob/master/.github/actions/ngen-submod-build/action.yaml> to build (sub)module libraries. Although convenient because the set-up already exists, this is not exactly what we want because action pulls whatever pinned commit is currently in ngen/extern from the submodules master/main branch. So we aren't even running the integration workflow with updated code from PR. At glance, I'm thinking ${{ github.head_ref }} maybe worth investigating... 2. Because the NOM build <https://github.com/NOAA-OWP/ngen/blob/f91e2ea9dbce706a2faa6f04d0600c2522db6006/extern/noah-owp-modular/CMakeLists.txt#L30> requires a -DNGEN_IS_MAIN_PROJECT flag, the use of the linked action in (1) to build NOM specifically, now breaks as ngen issue #761 <http://NGEN_IS_MAIN_PROJECT> addresses. See proposed ngen pr #768 <NOAA-OWP/ngen#768> if this helps clarify. @hellkite500 <https://github.com/hellkite500> can correct me if I'm off. I believe getting item (1), this issue #104 <#104>, is more priority. — Reply to this email directly, view it on GitHub <#104 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACA4SRI3YM7IAWE35VOGF3LYZB7Y3AVCNFSM6AAAAABE4N36P6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMBXHA3DIMRZGU> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[stcui007]

Looks like this is not an across all submodule problem for workflows/ngen-integration. But, at least another one, Topmodel, possibly PET, may need to be fixed. On Tue, Mar 19, 2024 at 5:19 PM Shengting Cui - NOAA Affiliate < ***@***.***> wrote:

…

It looked like this line of codes in ngen-submodule-build/action.yaml: run: git submodule update --init --recursive -- ${{ inputs.mod-dir }} over wrote the newest version we checked out earlier and moved back to extern/. We should build the submodule before checkout ngen and move the already built sub-module to extern, not build using ngen submodule action. On Tue, Mar 19, 2024 at 1:29 PM JessicaGarrett-NOAA < ***@***.***> wrote: > @hellkite500 <https://github.com/hellkite500>, other ngen-build > workflows are failing (e.g., in evapotranspiration > <https://github.com/NOAA-OWP/evapotranspiration/actions/runs/8299846846/job/22716483404?pr=31>). > Do you think this is the cause across formulations? > > So, there's kind of two unique-ish things going on here, > > 1. This issue Nels just outline. Most (if not all?) > ngen_integration.yml workflows (across all implemented formation > repos) directly call an existing 'ngen' action > <https://github.com/NOAA-OWP/ngen/blob/master/.github/actions/ngen-submod-build/action.yaml> > to build (sub)module libraries. Although convenient because the set-up > already exists, this is not exactly what we want because action pulls > whatever pinned commit is currently in ngen/extern from the > submodules master/main branch. So we aren't even running the > integration workflow with updated code from PR. At glance, I'm thinking ${{ > github.head_ref }} maybe worth investigating... > 2. Because the NOM build > <https://github.com/NOAA-OWP/ngen/blob/f91e2ea9dbce706a2faa6f04d0600c2522db6006/extern/noah-owp-modular/CMakeLists.txt#L30> > requires a -DNGEN_IS_MAIN_PROJECT flag, the use of the linked action > in (1) to build NOM specifically, now breaks as ngen issue #761 > <http://NGEN_IS_MAIN_PROJECT> addresses. See proposed ngen pr #768 > <NOAA-OWP/ngen#768> if this helps clarify. > > @hellkite500 <https://github.com/hellkite500> can correct me if I'm off. > I believe getting item (1), this issue #104 > <#104>, is more > priority. > > — > Reply to this email directly, view it on GitHub > <#104 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/ACA4SRI3YM7IAWE35VOGF3LYZB7Y3AVCNFSM6AAAAABE4N36P6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMBXHA3DIMRZGU> > . > You are receiving this because you are subscribed to this thread.Message > ID: ***@***.***> >

[madMatchstick]

ok, I'll take a look. Thanks @stcui007 !