Skip to content

config_file: refresh when creating an iterator#5181

Merged
ethomson merged 4 commits intolibgit2:masterfrom
pks-t:pks/config-iterator-refresh
Jul 22, 2019
Merged

config_file: refresh when creating an iterator#5181
ethomson merged 4 commits intolibgit2:masterfrom
pks-t:pks/config-iterator-refresh

Conversation

@pks-t
Copy link
Copy Markdown
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
Copy Markdown
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