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

cli: make paths to auto-track configurable, add jj track #4338

Merged
merged 1 commit into from
Sep 9, 2024

Conversation

martinvonz
Copy link
Owner

#323

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added tests to cover my changes

cli/src/cli_util.rs Outdated Show resolved Hide resolved
lib/src/local_working_copy.rs Outdated Show resolved Hide resolved
docs/working-copy.md Outdated Show resolved Hide resolved
cli/src/commands/track.rs Outdated Show resolved Hide resolved
@martinvonz martinvonz marked this pull request as ready for review August 25, 2024 18:00
CHANGELOG.md Outdated Show resolved Hide resolved
docs/working-copy.md Outdated Show resolved Hide resolved
cli/src/commands/file/track.rs Outdated Show resolved Hide resolved
cli/src/commands/file/track.rs Outdated Show resolved Hide resolved
lib/src/local_working_copy.rs Outdated Show resolved Hide resolved
lib/src/local_working_copy.rs Outdated Show resolved Hide resolved
@martinvonz martinvonz force-pushed the push-yroxsvpsklzk branch 2 times, most recently from f6c20d7 to 7af58bb Compare August 26, 2024 19:53
CHANGELOG.md Outdated Show resolved Hide resolved
docs/working-copy.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@ilyagr ilyagr left a comment

Choose a reason for hiding this comment

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

Nice! I only glanced at the docs, but this seems like a nice idea to try.

docs/FAQ.md Outdated Show resolved Hide resolved
docs/working-copy.md Outdated Show resolved Hide resolved
@ilyagr
Copy link
Collaborator

ilyagr commented Sep 2, 2024

I'll also Cc #4188 for the Windows error. In the case of this PR, the solution might be simpler, though.

@ilyagr ilyagr mentioned this pull request Sep 3, 2024
3 tasks
CHANGELOG.md Outdated
@@ -20,6 +20,12 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

### New features

* The new config option `snapshot.auto-track` lets you automatically track only
Copy link
Collaborator

@ilyagr ilyagr Sep 3, 2024

Choose a reason for hiding this comment

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

(Not necessarily blocking, there are likely devils in the details even if this is a good idea, and this could be implemented later) I just had this possibly crazy idea: could we replace this option with a sparse pattern? Do we need separate concepts for sparse patterns and auto-tracked files?

jj file (un)track could modify sparse patterns for the working copy (see below, #4338 (comment)). We could then rename "sparse patterns" if that's useful, and we'd probably need to adjust the UI for them.

Much of the other discussion in this PR (e.g. #4338 (comment)) would still apply.

I had this thought after writing the update to #4338 (comment). I could be missing something, of course, about the technical consequences or about the goals of the PR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The main issue that I can think of with spare files is that it's possible that the file X is in the repo and outside the sparse set, and there is a different X in the working copy. I'm not sure what is best to do in this case, but complaining loudly and treating this like a conflict of sorts seems like an option.

Copy link
Collaborator

@ilyagr ilyagr Sep 3, 2024

Choose a reason for hiding this comment

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

jj file untrack would remove the file from the repo and from the sparse pattern, but would keep the file in the working copy. There would also be a way to change the sparse pattern without changing the repo, which would remove the file from the working copy (this is probably what jj sparse edit already does).

One issue is whether we'd still try to warn people if they jj file untrack a file that's not .gitignore-d. This would be more difficult with the sparse files approach as opposed to the snapshot.auto-track approach (with this PR's approach, what happens depends on whether the file is matches the special snapshot.auto-track pattern).

Copy link
Collaborator

@ilyagr ilyagr Sep 3, 2024

Choose a reason for hiding this comment

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

Another issue here is whether sparse patterns can handle (currently, or whether they can be sensibly changed to handle) gitignored-but-still-tracked files.

Update: I think they can, since we can have such files in practice right now.

Copy link
Owner Author

Choose a reason for hiding this comment

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

So for people who want snapshot.auto-track = "none()", would would they do if we had only sparse patterns? Would they include no paths? But that would mean that they would not get any files from the repo mapped to the working copy. What am I missing?

Copy link
Owner Author

Choose a reason for hiding this comment

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

I'll mark this resolved so I can merge this PR now. We can revisit it later.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Making this function properly might be a bit trickier than I initially thought, but I think it might still be worth thinking about.

The analogue to snapshot.auto-track = "none()" would be to have the sparse patterns exclude any files that are not in the working copy commit. If this set was constant, that would be it; the "trickier than I initially thought" part is that currently the sparse set is the property of the working copy, while the set of files in the commit depends on the commit.

Even less conclusively, perhaps it's inevitable to have two separate configs for patters that are explicitly added to the sparse files, and patterns that are explicitly subtracted, but even then it might be useful to think in terms of these as methods to determine a single final set of files in the working copy that are tracked + snapshotted.

@martinvonz martinvonz force-pushed the push-yroxsvpsklzk branch 5 times, most recently from 1a18577 to 7ad4d38 Compare September 7, 2024 16:18
CHANGELOG.md Outdated Show resolved Hide resolved
@martinvonz martinvonz force-pushed the push-yroxsvpsklzk branch 2 times, most recently from a1b4f29 to 2817cbc Compare September 7, 2024 20:16
It's a pretty frequent request to have support for turning off
auto-tracking of new files and to have a command to manually track
them instead. This patch adds a `snapshot.auto-track` config to decide
which paths to auto-track (defaults to `all()`). It also adds a `jj
track` command to manually track the untracked paths.

This patch does not include displaying the untracked paths in `jj
status`, so for now this is probably only useful in colocated repos
where you can run `git status` to find the untracked files.

#323
@martinvonz martinvonz merged commit f36f4ad into main Sep 9, 2024
31 checks passed
@martinvonz martinvonz deleted the push-yroxsvpsklzk branch September 9, 2024 14:49
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.

8 participants