Skip to content

fix: add NoCache option to host directory uploads #96

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

grouville
Copy link
Member

@grouville grouville commented Jun 17, 2025

Rebased on top of #65

> Maybee it is being fixed by the new environment

Summary

  • Fixes caching issue where uploading modified local directories would use stale cached versions
  • Adds NoCache: true to all dag.Host().Directory() calls in urlToDirectory()
  • Enables and updates integration test that was previously skipped due to this issue

Test plan

  • Enabled and fixed TestUploadAfterModification integration test
  • Test passes with the fix, fails without it (verified)
  • Added edge case documentation for worktree subdirectory uploads (if users think it still makes sense) -> users might want to change something on the worktree, what's the expected behavior ?

…al behaviors users rely on:

* Persistence of work across sessions (files and changes remain intact through restarts).
* Automatic change tracking and audit trails for effective debugging.
* Graceful handling of problematic files, including Python cache, binary files, and large files.
* Isolation of multiple environments to support safe parallel operations.
* Reliable persistence of environment configuration (base images, setup commands).

Detailed testing includes:

* Git operations, specifically handling command errors, worktree paths, and empty directories.
* Selective file staging to manage Python cache and binary files appropriately.
* Verification of configuration persistence and environment isolation.

Adopted a behavior-driven testing approach focusing on user-experienced behaviors rather than internal implementation details.

Modified git.go to support test isolation via the `CONTAINER_USE_CONFIG_DIR` environment variable. Introduced test helpers to streamline the creation of isolated test environments.

Documented known limitations clearly with skipped tests for currently unresolved issues related to Python cache, binary directories, and environment variables.

Signed-off-by: Guillaume de Rouville <[email protected]>
make it work

Signed-off-by: Guillaume de Rouville <[email protected]>
Signed-off-by: Guillaume de Rouville <[email protected]>
Updated the test to use commands that developers actually run:
- Creating build directories (common in CI/CD)
- Writing to build output logs (typical build process)
- Creating coverage directories (test runners like pytest)

These are more representative of real-world scenarios where commands
produce no git-trackable changes but should still be audited.

Related to issue dagger#82
The test now:
- Simulates Python cache directories without needing Python installed
- Verifies that development continues normally with __pycache__ present
- Serves as a regression test for Python workflows
- Renamed from PythonCache to PythonDevelopmentWorkflow for clarity

The original bug appears to have been fixed - __pycache__ directories
are properly ignored by git and don't interfere with operations.
Concurrency was properly implemented -- added a test showing that in a sequencial environment, every git operation is sequential and working
Removed the impossible concurrency test

Signed-off-by: Guillaume de Rouville <[email protected]>
Signed-off-by: Guillaume de Rouville <[email protected]>
Signed-off-by: Guillaume de Rouville <[email protected]>
Previously, when uploading local directories to container environments,
Dagger would cache the directory contents. This caused subsequent uploads
to use stale cached versions instead of the latest local changes.

This fix adds NoCache: true to all dag.Host().Directory() calls to ensure
fresh reads of local files on each upload.

Also enables and updates the TestUploadAfterModification integration test
to verify this behavior works correctly.

Signed-off-by: Guillaume de Rouville <[email protected]>
@cwlbraa
Copy link
Collaborator

cwlbraa commented Jun 18, 2025

the upload tools fell out in the repository refactor, so this will need to be reexamined.

that said:

i'm not entirely sure this is the right way to fix the test... NoCache definitely gives correct behavior for Upload, but i'm fairly certain that url transformer was also used for environment_source? (keyword was)

if that's true, the most recent container should have all the accumulated changes from the run_command and file_write tools, right? so busting the cache and re-uploading the base directory with those changes creates a weird situation where we then go and re-apply those writes on top of a different base.

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.

2 participants