Honor read-only flag when writing to config backends#4217
Merged
Conversation
The config_file.h header provides some inline declarations accessing the `git_config_backend`, but misses its declaration. Add the missing include for "git2/sys/config.h" to add it.
ethomson
reviewed
Apr 25, 2017
|
|
||
| if (git_vector_length(&cfg->files) == 0) { | ||
| giterr_set(GITERR_CONFIG, | ||
| "cannot set value for '%s' when no config files exist", name); |
Member
There was a problem hiding this comment.
If I can bikeshed here just a touch, it might be nice to switch this message on whether we're in a set function or in delete_entry.
Configuration backends have a readonly-flag which is currently used to distinguish configuration snapshots. But somewhat unexpectedly, we do not use the flag to prevent writing to a readonly backend but happily proceed to do so. This commit modifies logic to also honor the readonly flag for configuration setters. We will now traverse through all backends and pick the first one which is not marked as read-only whenever we want to write new configuration.
pks-t
force-pushed
the
pks/readonly-cfg-backend
branch
from
894eda2 to
2a7086f
Compare
Member
Author
|
Okay, I've extended the function to accept a use-case. Right now it feels a bit overengineered, so I initially thought about extending the function to also handle the other use cases (lock/unlock and read). It would then be some kind of "get me the right thing for whatever I want to do" function, so it would also honor the readonly flag. But the interface would look horrible. So I refrained from doing so and instead stayed with this version. |
Member
|
Seems fine to me. Thanks! |
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Configuration backends have a readonly-flag which is currently used to
distinguish configuration snapshots. But somewhat unexpectedly, we do
not use the flag to prevent writing to a readonly backend but happily
proceed to do so.
This commit modifies logic to also honor the readonly flag for
configuration setters. We will now traverse through all backends and
pick the first one which is not marked as read-only whenever we want to
write new configuration.
Implements #4134