Skip to content

Conversation

@obbardc
Copy link
Member

@obbardc obbardc commented Aug 15, 2023

Currently any run action tagged as postprocess will be ran in the recipe
directory. The artifacts are stored in this directory by default, unless
--artifactdir is passed to Debos to change where the artifacts are stored.

The run action documentation states:

postprocess -- if set script or command is executed after all other
commands and has access to the recipe directory ($RECIPEDIR) and the
artifact directory ($ARTIFACTDIR). The working directory will be set to
the artifact directory.

But this is wrong; currently the working directory of postprocess commands
is set to the recipe directory. Set the working directory to the artifact
directory instead to allow postprocess commands to be ran in the correct
location.

Fixes: #345
Closes: #617
Closes: #618
Closes: #619

@obbardc obbardc requested a review from sjoerdsimons August 15, 2023 10:25
@obbardc obbardc self-assigned this Aug 15, 2023
@evelikov
Copy link

evelikov commented Oct 3, 2023

Adding the reproducer (or variant thereof) as a test case would be great.

@obbardc obbardc force-pushed the wip/obbardc/artifactdir-postprocess branch from be044e5 to c71008f Compare January 10, 2024 14:33
@obbardc obbardc added this to the v1.1.4 milestone Jan 10, 2024
@obbardc obbardc force-pushed the wip/obbardc/artifactdir-postprocess branch from c71008f to c43257e Compare January 10, 2024 15:20
Copy link
Member

@sjoerdsimons sjoerdsimons left a comment

Choose a reason for hiding this comment

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

We should really ensure this get exercised in at least one of the tests

@sjoerdsimons
Copy link
Member

@obbardc will you have to time to nudge this forward? the outstanding comments were quite small

@obbardc
Copy link
Member Author

obbardc commented Feb 16, 2025

@obbardc will you have to time to nudge this forward? the outstanding comments were quite small

yes, I will be able to take a look at the beginning of March, along with my other outstanding PRs :-)

@obbardc obbardc moved this from Review required to Stalled in Debos&Fakemachine development Aug 8, 2025
@obbardc obbardc force-pushed the wip/obbardc/artifactdir-postprocess branch from c43257e to c35d190 Compare August 10, 2025 15:56
@obbardc
Copy link
Member Author

obbardc commented Aug 10, 2025

Not sure what test I can write for this. Other than a run action with postprocess confirming the artifact dir. Any ideas @sjoerdsimons @daissi ?

@obbardc obbardc moved this from Stalled to In progress in Debos&Fakemachine development Aug 10, 2025
@sjoerdsimons
Copy link
Member

Not sure what test I can write for this. Other than a run action with postprocess confirming the artifact dir. Any ideas @sjoerdsimons @daissi ?

I think that's it right? Just confirming the postprocess cwd matches what you'd expect ? e.g. by writing an artifact in an action and then checking you can access in a postprocessing step

@obbardc obbardc force-pushed the wip/obbardc/artifactdir-postprocess branch 3 times, most recently from bb12b78 to 273b628 Compare August 11, 2025 15:18
Copy link
Contributor

Copilot AI commented Nov 28, 2025

@sjoerdsimons I've opened a new pull request, #617, to work on those changes. Once the pull request is ready, I'll request review from you.

@obbardc obbardc force-pushed the wip/obbardc/artifactdir-postprocess branch from 9bc87c5 to e8a244b Compare November 28, 2025 21:28
Copy link
Contributor

Copilot AI commented Nov 29, 2025

@sjoerdsimons I've opened a new pull request, #618, to work on those changes. Once the pull request is ready, I'll request review from you.

Copy link
Contributor

Copilot AI commented Nov 29, 2025

@sjoerdsimons I've opened a new pull request, #619, to work on those changes. Once the pull request is ready, I'll request review from you.

@obbardc
Copy link
Member Author

obbardc commented Nov 30, 2025

I've added the changes from #618 and #619 as a draft commit to see if the test works fine in CI. It will need squashing once it works.

@obbardc obbardc force-pushed the wip/obbardc/artifactdir-postprocess branch from 7c878eb to 7d405ca Compare November 30, 2025 21:10
@obbardc obbardc requested a review from sjoerdsimons December 1, 2025 17:31
@sjoerdsimons sjoerdsimons force-pushed the wip/obbardc/artifactdir-postprocess branch from 7d405ca to 0ac3a4e Compare December 4, 2025 19:30
@obbardc obbardc force-pushed the wip/obbardc/artifactdir-postprocess branch from 0ac3a4e to f4c7e4b Compare December 4, 2025 21:14
@sjoerdsimons sjoerdsimons force-pushed the wip/obbardc/artifactdir-postprocess branch from f4c7e4b to 1ea917f Compare December 5, 2025 20:48
jq is needed for some tests but is no longer installed since the container
base image bump to trixie. Install jq.

Signed-off-by: Christopher Obbard <[email protected]>
The partition UUIDs are not reproducible between runs; remove the UUIDs
from the check such that any changes in the partition UUIDs don't cause
the tests to fail.

Signed-off-by: Christopher Obbard <[email protected]>
Currently Command.Dir is unsed; hook it up such that commands can
have a working directory set.

Signed-off-by: Christopher Obbard <[email protected]>
Currently any run action tagged as postprocess will be ran in the recipe
directory. The artifacts are stored in this directory by default, unless
--artifactdir is passed to Debos to change where the artifacts are stored.

The run action documentation states:

  postprocess -- if set script or command is executed after all other
  commands and has access to the recipe directory ($RECIPEDIR) and the
  artifact directory ($ARTIFACTDIR). The working directory will be set to
  the artifact directory.

But this is wrong; currently the working directory of postprocess commands
is set to the recipe directory. Set the working directory to the artifact
directory instead to allow postprocess commands to be ran in the correct
location.

Fixes: #345
Signed-off-by: Christopher Obbard <[email protected]>
@obbardc obbardc force-pushed the wip/obbardc/artifactdir-postprocess branch from 1ea917f to 8993ee5 Compare December 7, 2025 22:26
@obbardc obbardc disabled auto-merge December 7, 2025 22:26
@obbardc
Copy link
Member Author

obbardc commented Dec 7, 2025

@sjoerdsimons added some fixes for the partitioning test; hope it's ok. Disabled auto merge so you can review before merging... Cheers!

@obbardc obbardc requested a review from sjoerdsimons December 7, 2025 22:27
@obbardc
Copy link
Member Author

obbardc commented Dec 7, 2025

For some reason it fails as the image can't be found in artifactdir in CI (but of course it works locally). Added some test commits to check...

hmm, in the CI run, ls -la $ARTIFACTDIR returns:

2025/12/07 23:19:30 ==== test ====
2025/12/07 23:19:30 ls -la ${ARTIFACTDIR} | total 4
2025/12/07 23:19:30 ls -la ${ARTIFACTDIR} | drwxrwxrwt  4 root root   80 Dec  7 23:19 .
2025/12/07 23:19:30 ls -la ${ARTIFACTDIR} | drwxrwxrwt 15 root root  440 Dec  7 23:19 ..
2025/12/07 23:19:30 ls -la ${ARTIFACTDIR} | drwxr-xr-x  3 root root 4096 Dec  7 23:19 mnt
2025/12/07 23:19:30 ls -la ${ARTIFACTDIR} | drwxr-xr-x  2 root root   40 Dec  7 23:19 root

Extend the partitioning test to enforce the expected working directory
behaviour for postprocess actions. When a recipe is executed with an
explicit --artifactdir, postprocess run actions should execute with:

 * cwd == $ARTIFACTDIR
 * $ARTIFACTDIR != $RECIPEDIR

The test now checks all three conditions:
 * test.img exists in the current directory,
 * the working directory is $ARTIFACTDIR,
 * that $ARTIFACTDIR is different from $RECIPEDIR.

This ensures correct isolation between recipe sources and build
artifacts and catches regressions where postprocess commands
incorrectly inherit the recipe directory as their working directory.

Signed-off-by: Christopher Obbard <[email protected]>
@obbardc obbardc force-pushed the wip/obbardc/artifactdir-postprocess branch from 3593662 to fe0599f Compare December 8, 2025 00:26
@obbardc
Copy link
Member Author

obbardc commented Dec 8, 2025

For some reason it fails as the image can't be found in artifactdir in CI (but of course it works locally). Added some test commits to check...

hmm, in the CI run, ls -la $ARTIFACTDIR returns:

2025/12/07 23:19:30 ==== test ====
2025/12/07 23:19:30 ls -la ${ARTIFACTDIR} | total 4
2025/12/07 23:19:30 ls -la ${ARTIFACTDIR} | drwxrwxrwt  4 root root   80 Dec  7 23:19 .
2025/12/07 23:19:30 ls -la ${ARTIFACTDIR} | drwxrwxrwt 15 root root  440 Dec  7 23:19 ..
2025/12/07 23:19:30 ls -la ${ARTIFACTDIR} | drwxr-xr-x  3 root root 4096 Dec  7 23:19 mnt
2025/12/07 23:19:30 ls -la ${ARTIFACTDIR} | drwxr-xr-x  2 root root   40 Dec  7 23:19 root

OK, fixed it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

actions: run: Postprocess working directory should be set to the artifact directory

4 participants