Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Separate config reader and writer backend priorities (for worktree configs)#6756

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
ethomson merged 19 commits intomainfromethomson/worktree-config
Mar 16, 2024

Conversation

ethomson
Copy link
Member

For#6202, we add additional customization on read/write priority, so that we can have independent read and write ordering. This allows us to send writes to the local configuration backendeven if there is a higher-priority read backend (like a worktree configuration). Repositories will configure the read/write order specifically, and users can add aAPP level configuration which takes priority (over both). Users can further tweak priority themselves.

vermiculusand others added11 commitsFebruary 20, 2024 11:30
Introduce the logical concept of a worktree-level config.  The newvalue sits between _LOCAL and _APP to allow `git_config_get_*` to'just work'.The assumption of how `git_config_get_*` works was testedexperimentally by setting _WORKTREE to some nonsense value (like -3)and watching the new test fail.
Now that GIT_CONFIG_LEVEL_WORKTREE exists logically, define and load$GIT_DIR/config.worktree into that level.
Now that worktree-level configuration can be read from disk andmanipulated in memory, we should be able to say we support'extensions.worktreeConfig'.
This structure provides for cleaner diffs in upcoming commits.
It would seem that `get_backend_for_use` is primarily used whenwriting config data -- either to set keys or delete them (based on thepossible values of `backend_use`). When git-config(1) is used forside-effects, it will modify only the local (repository-level)configuration unless explicitly overridden.From git-config(1):    --local        For writing options: write to the repository .git/config file.This is the default behavior.`get_backend_for_use` does not have the ability to specify a configlevel and typically is expected (it seems) to 'do the right thing'.Taking its cue from git-config(1), don't update worktree-specificconfig unless it's the only option.If that functionality is needed by consumers, I assume they would findthe appropriate backend with `git_config_open_level` and feed that`git_config` object through to the `git_config_set_*` functions (asdemonstrated in the provided test).
When deleting a key from a repository with multiple configbackends (like a --local config and a --worktree config), it'simportant to return the correct backend to modify.This patch ensures that we don't return a backend that is incapable ofdeleting a given piece of configuration when that is the required use.
Before passing the config key name to backend->get(), it needs to benormalized first. Not all current callers are performing thisnormalization, so perform it centrally instead.Co-authored-by: Brian Lyles <brianmlyles@gmail.com>
This looks like a simple copy/paste error.
Now that get_backend_for_use can return other error codes (by virtueof key-name normalization), make sure to propagate the appropriateerror code when used.
`cl_git_fail_with` is preferred over asserting a return value; theformer gives more detailed information about the mismatch.
@ethomsonethomsonforce-pushed theethomson/worktree-config branch fromb78bb4f to1fcc5deCompareMarch 10, 2024 21:19
Configuration read order and write order should be separated. Forexample: configuration readers have a worktree level that is higherpriority than the local configuration _for reads_. Despite that, theworktree configuration is not written to by default.Use a new list, `writers`, to identify the write target.To do this, we need another level of indirection between backendinstances (which are refcounted and shared amongst different git_configinstances) and the config reader/writer list (since each of thosedifferent git_config instances can have different read/writepriorities).
Introduce `GIT_EREADONLY` and return it when a read-only configurationis attempted to be mutated. This is preferred over the prior`GIT_ENOTFOUND` which is not accurate.
When a configuration transaction is freed with `git_transaction_free` -without first committing it - we should not `git_config_free`. We neverincreased the refcount, so we certainly shouldn't decrease it.Also, add a test around aborting a transaction without committing it.
When we `git_config_unlock`, we shouldn't _unlock the thing that welocked_ instead of assuming that the highest-priority target _remains_the highest-priority target.
When we start a transaction, we should refcount the backend so that wedon't lose it. This prevents the case where we have a transaction openand a configuration entry locked and somebody forces a new backend atthe same level and the backend is freed. Now a user can gently wind downtheir transaction even when the backend has been removed from the liveconfiguration object.
@ethomsonethomsonforce-pushed theethomson/worktree-config branch from1fcc5de toe8910c1CompareMarch 10, 2024 21:35
@ethomson
Copy link
MemberAuthor

ethomson commentedMar 11, 2024
edited
Loading

It looks like we support reading theworktreeconfig extension — but it looks like we're always supporting the worktree configeven if the worktreeconfig extension isn't specified. We should validate that.

Fixed!

Only read the worktree configuration when `extensions.worktreeconfig` isset to true.
Instead of reading the worktree configuration from the commondir, readit from the gitdir. This ensures that each worktree gets its ownconfiguration.
@ethomson
Copy link
MemberAuthor

cc@vermiculus - curious if you have any feedback here.

@ethomson
Copy link
MemberAuthor

Going to merge this, I think it’s a nice little api change. Thanks again@vermiculus

@ethomsonethomson merged commitc2ed49c intomainMar 16, 2024
@ethomsonethomson deleted the ethomson/worktree-config branchMarch 16, 2024 16:36
@zivarah
Copy link
Contributor

cc@vermiculus - curious if you have any feedback here.

@ethomson apologies for the lack of response, both myself and@vermiculus got a little busy and this slipped through. Thanks for running with this!

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

3 participants
@ethomson@zivarah@vermiculus

[8]ページ先頭

©2009-2025 Movatter.jp