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.

Sign up for GitHub

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

Jump to bottom

deserialize-status: silently fallback if we cannot read cache file #270

Merged
merged 1 commit into from
May 14, 2020
Merged

deserialize-status: silently fallback if we cannot read cache file #270

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 12 additions & 6 deletions builtin/commit.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,12 +211,18 @@ static int opt_parse_deserialize(const struct option *opt, const char *arg, int
free(deserialize_path);
deserialize_path = xstrdup(arg);
}
if (deserialize_path && *deserialize_path
&& (wt_status_deserialize_access(deserialize_path, R_OK) != 0))
die("cannot find serialization file '%s'",
deserialize_path);

do_explicit_deserialize = 1;
if (!deserialize_path || !*deserialize_path)
do_explicit_deserialize = 1; /* read stdin */
else if (wt_status_deserialize_access(deserialize_path, R_OK) == 0)
do_explicit_deserialize = 1; /* can read from this file */
Comment on lines +215 to +217
Copy link
Collaborator

Choose a reason for hiding this comment

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

the fact that these two variables are the same made me wonder if this is the right approach, but you seem to test both cases.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Naming things is hard.

The variable do_implicit... handles the case where we inherited a config setting. This was something that the gvfs installer set up and is usually outside of the user's view.

The do_explicit... variable handles the --deserialize[=<path>] command line option, which overrides the config setting. IIRC this was added later.

The original design was that if the path from the config was not found, silently ignore it and go on and do a scan -- since that would happen any time the gvfs service was having problems, for example, and the user wouldn't know why status was broken and they were getting an obscure error message.

But when given on the command line, usual practice is to complain if an input is not found.
But that hard error threw Jameson's test/expectations.

Hence the 2 different code paths in the original code for determining whether we can read the status-cache and whether to complain about it.

Structurally, the actual code change is a lot smaller than it looks, but I refactored it a bit for (hopefully) clarity. Perhaps I should have written it as:

if (path && *path && access(...)) {
    do_implicit = 0
    do_explicit = 0
    return 0
}

else {
/*
* otherwise, silently fallback to the normal
* collection scan
*/
do_implicit_deserialize = 0;
do_explicit_deserialize = 0;
}
}

return 0;
Expand Down
16 changes: 16 additions & 0 deletions t/t7524-serialized-status.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -426,4 +426,20 @@ test_expect_success 'ensure deserialize -v does not crash' '
grep -q "deserialize/reject:args/verbose" verbose_test.log_v
'

test_expect_success 'fallback when implicit' '
git init implicit_fallback_test &&
git -C implicit_fallback_test -c status.deserializepath=foobar status
'

test_expect_success 'fallback when explicit' '
git init explicit_fallback_test &&
git -C explicit_fallback_test status --deserialize=foobar
'

test_expect_success 'deserialize from stdin' '
git init stdin_test &&
git -C stdin_test status --serialize >serialized_status.dat &&
cat serialize_status.dat | git -C stdin_test status --deserialize
'

test_done