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

config_file: refresh when creating an iterator #5181

Merged
merged 4 commits into from
Jul 22, 2019

Conversation

pks-t
Copy link
Member

@pks-t pks-t commented Jul 21, 2019

When creating a new iterator for a config file backend, then we should
always make sure that we're up to date by calling config_refresh.
Otherwise, we might not notice when another process has modified the
configuration file and thus will represent outdated values.

Add two tests to config::stress that verify that we get up-to-date
values when reading configuration entries via git_config_iterator.


I think this wasn't caused by #5132 but was a pre-existing issue already. There simply was no code path that would've called config_refresh when creating a new iterator.

Fixes #5167

pks-t and others added 4 commits July 21, 2019 14:41
If calling `config_refresh` on a read-only configuration file backend,
then we will segfault when comparing the timestamp of the file due to
`path` being uninitialized. As a read-only snapshot should not be
refreshed anyway and stay consistent, we can simply return early when
calling `config_refresh` on a read-only snapshot.
When creating a new iterator for a config file backend, then we should
always make sure that we're up to date by calling `config_refresh`.
Otherwise, we might not notice when another process has modified the
configuration file and thus will represent outdated values.

Add two tests to config::stress that verify that we get up-to-date
values when reading configuration entries via `git_config_iterator`.
There was a bug when calling `git_remote_list` that caused us to not
re-read modified configurations when using `git_config_iterator`. This
bug also impacted `git_remote_list`, which thus failed to provide an
up-to-date list of remotes. Add a test suite remote::list with a single
test that verifies we do the right thing.
@ethomson
Copy link
Member

Thanks much for the helpful report and the very nice tests @Mr-Wallet!

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.

git_remote_list does not detect config changes on disk
3 participants