update-server-info: avoid needless overwrites
Do not change the existing info/refs and objects/info/packs files if they match the existing content on the filesystem. This is intended to preserve mtime and make it easier for dumb HTTP pollers to rely on the If-Modified-Since header. Combined with stdio and kernel buffering; the kernel should be able to avoid block layer writes and reduce wear for small files. As a result, the --force option is no longer needed. So stop documenting it, but let it remain for compatibility (and debugging, if necessary). v3: perform incremental comparison while generating to avoid OOM with giant files. Remove documentation for --force. Signed-off-by: Eric Wong <e@80x24.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Eric Wong committed
May 13, 2019 at 23:17 UTC
f4f476b6a19217b6ed3d5022422b3fa0f55a5ee9
3 files changed
+158
-34
Documentation/git-update-server-info.txt
+1
-10
@@ -9,7 +9,7 @@ git-update-server-info - Update auxiliary info file to help dumb servers
9
SYNOPSIS
10
--------
11
[verse]
12
-'git update-server-info' [--force]
12
+'git update-server-info'
13
14
DESCRIPTION
15
-----------
@@ -19,15 +19,6 @@ $GIT_OBJECT_DIRECTORY/info directories to help clients discover
19
what references and packs the server has. This command
20
generates such auxiliary files.
21
22
-
23
-OPTIONS
24
--------
25
-
26
--f::
27
---force::
28
- Update the info files from scratch.
29
-
30
-
22
OUTPUT
23
------
24
server-info.c
+116
-24
@@ -6,82 +6,174 @@
6
#include "tag.h"
7
#include "packfile.h"
8
#include "object-store.h"
9
+#include "strbuf.h"
10
+
11
+struct update_info_ctx {
12
+ FILE *cur_fp;
13
+ FILE *old_fp; /* becomes NULL if it differs from cur_fp */
14
+ struct strbuf cur_sb;
15
+ struct strbuf old_sb;
16
+};
17
+
18
+static void uic_mark_stale(struct update_info_ctx *uic)
19
+{
20
+ fclose(uic->old_fp);
21
+ uic->old_fp = NULL;
22
+}
23
+
24
+static int uic_is_stale(const struct update_info_ctx *uic)
25
+{
26
+ return uic->old_fp == NULL;
27
+}
28
+
29
+static int uic_printf(struct update_info_ctx *uic, const char *fmt, ...)
30
+{
31
+ va_list ap;
32
+ int ret = -1;
33
+
34
+ va_start(ap, fmt);
35
+
36
+ if (uic_is_stale(uic)) {
37
+ ret = vfprintf(uic->cur_fp, fmt, ap);
38
+ } else {
39
+ ssize_t r;
40
+ struct strbuf *cur = &uic->cur_sb;
41
+ struct strbuf *old = &uic->old_sb;
42
+
43
+ strbuf_reset(cur);
44
+ strbuf_vinsertf(cur, 0, fmt, ap);
45
+
46
+ strbuf_reset(old);
47
+ strbuf_grow(old, cur->len);
48
+ r = fread(old->buf, 1, cur->len, uic->old_fp);
49
+ if (r != cur->len || memcmp(old->buf, cur->buf, r))
50
+ uic_mark_stale(uic);
51
+
52
+ if (fwrite(cur->buf, 1, cur->len, uic->cur_fp) == cur->len)
53
+ ret = 0;
54
+ }
55
+
56
+ va_end(ap);
57
+
58
+ return ret;
59
+}
60
61
/*
62
* Create the file "path" by writing to a temporary file and renaming
63
* it into place. The contents of the file come from "generate", which
64
* should return non-zero if it encounters an error.
65
*/
15
-static int update_info_file(char *path, int (*generate)(FILE *))
66
+static int update_info_file(char *path,
67
+ int (*generate)(struct update_info_ctx *),
68
+ int force)
69
{
70
char *tmp = mkpathdup("%s_XXXXXX", path);
71
int ret = -1;
72
int fd = -1;
20
- FILE *fp = NULL, *to_close;
73
+ FILE *to_close;
74
+ struct update_info_ctx uic = {
75
+ .cur_fp = NULL,
76
+ .old_fp = NULL,
77
+ .cur_sb = STRBUF_INIT,
78
+ .old_sb = STRBUF_INIT
79
+ };
80
81
safe_create_leading_directories(path);
82
fd = git_mkstemp_mode(tmp, 0666);
83
if (fd < 0)
84
goto out;
26
- to_close = fp = fdopen(fd, "w");
27
- if (!fp)
85
+ to_close = uic.cur_fp = fdopen(fd, "w");
86
+ if (!uic.cur_fp)
87
goto out;
88
fd = -1;
30
- ret = generate(fp);
89
+
90
+ /* no problem on ENOENT and old_fp == NULL, it's stale, now */
91
+ if (!force)
92
+ uic.old_fp = fopen_or_warn(path, "r");
93
+
94
+ /*
95
+ * uic_printf will compare incremental comparison aginst old_fp
96
+ * and mark uic as stale if needed
97
+ */
98
+ ret = generate(&uic);
99
if (ret)
100
goto out;
33
- fp = NULL;
101
+
102
+ /* new file may be shorter than the old one, check here */
103
+ if (!uic_is_stale(&uic)) {
104
+ struct stat st;
105
+ long new_len = ftell(uic.cur_fp);
106
+ int old_fd = fileno(uic.old_fp);
107
+
108
+ if (new_len < 0) {
109
+ ret = -1;
110
+ goto out;
111
+ }
112
+ if (fstat(old_fd, &st) || (st.st_size != (size_t)new_len))
113
+ uic_mark_stale(&uic);
114
+ }
115
+
116
+ uic.cur_fp = NULL;
117
if (fclose(to_close))
118
goto out;
36
- if (adjust_shared_perm(tmp) < 0)
37
- goto out;
38
- if (rename(tmp, path) < 0)
39
- goto out;
119
+
120
+ if (uic_is_stale(&uic)) {
121
+ if (adjust_shared_perm(tmp) < 0)
122
+ goto out;
123
+ if (rename(tmp, path) < 0)
124
+ goto out;
125
+ } else {
126
+ unlink(tmp);
127
+ }
128
ret = 0;
129
130
out:
131
if (ret) {
132
error_errno("unable to update %s", path);
45
- if (fp)
46
- fclose(fp);
133
+ if (uic.cur_fp)
134
+ fclose(uic.cur_fp);
135
else if (fd >= 0)
136
close(fd);
137
unlink(tmp);
138
}
139
free(tmp);
140
+ if (uic.old_fp)
141
+ fclose(uic.old_fp);
142
+ strbuf_release(&uic.old_sb);
143
+ strbuf_release(&uic.cur_sb);
144
return ret;
145
}
146
147
static int add_info_ref(const char *path, const struct object_id *oid,
148
int flag, void *cb_data)
149
{
58
- FILE *fp = cb_data;
150
+ struct update_info_ctx *uic = cb_data;
151
struct object *o = parse_object(the_repository, oid);
152
if (!o)
153
return -1;
154
63
- if (fprintf(fp, "%s %s\n", oid_to_hex(oid), path) < 0)
155
+ if (uic_printf(uic, "%s %s\n", oid_to_hex(oid), path) < 0)
156
return -1;
157
158
if (o->type == OBJ_TAG) {
159
o = deref_tag(the_repository, o, path, 0);
160
if (o)
69
- if (fprintf(fp, "%s %s^{}\n",
161
+ if (uic_printf(uic, "%s %s^{}\n",
162
oid_to_hex(&o->oid), path) < 0)
163
return -1;
164
}
165
return 0;
166
}
167
76
-static int generate_info_refs(FILE *fp)
168
+static int generate_info_refs(struct update_info_ctx *uic)
169
{
78
- return for_each_ref(add_info_ref, fp);
170
+ return for_each_ref(add_info_ref, uic);
171
}
172
81
-static int update_info_refs(void)
173
+static int update_info_refs(int force)
174
{
175
char *path = git_pathdup("info/refs");
84
- int ret = update_info_file(path, generate_info_refs);
176
+ int ret = update_info_file(path, generate_info_refs, force);
177
free(path);
178
return ret;
179
}
@@ -236,14 +328,14 @@ static void free_pack_info(void)
328
free(info);
329
}
330
239
-static int write_pack_info_file(FILE *fp)
331
+static int write_pack_info_file(struct update_info_ctx *uic)
332
{
333
int i;
334
for (i = 0; i < num_pack; i++) {
243
- if (fprintf(fp, "P %s\n", pack_basename(info[i]->p)) < 0)
335
+ if (uic_printf(uic, "P %s\n", pack_basename(info[i]->p)) < 0)
336
return -1;
337
}
246
- if (fputc('\n', fp) == EOF)
338
+ if (uic_printf(uic, "\n") < 0)
339
return -1;
340
return 0;
341
}
@@ -254,7 +346,7 @@ static int update_info_packs(int force)
346
int ret;
347
348
init_pack_info(infofile, force);
257
- ret = update_info_file(infofile, write_pack_info_file);
349
+ ret = update_info_file(infofile, write_pack_info_file, force);
350
free_pack_info();
351
free(infofile);
352
return ret;
@@ -269,7 +361,7 @@ int update_server_info(int force)
361
*/
362
int errs = 0;
363
272
- errs = errs | update_info_refs();
364
+ errs = errs | update_info_refs(force);
365
errs = errs | update_info_packs(force);
366
367
/* remove leftover rev-cache file if there is any */
t/t5200-update-server-info.sh
new
+41
@@ -0,0 +1,41 @@
1
+#!/bin/sh
2
+
3
+test_description='Test git update-server-info'
4
+
5
+. ./test-lib.sh
6
+
7
+test_expect_success 'setup' 'test_commit file'
8
+
9
+test_expect_success 'create info/refs' '
10
+ git update-server-info &&
11
+ test_path_is_file .git/info/refs
12
+'
13
+
14
+test_expect_success 'modify and store mtime' '
15
+ test-tool chmtime =0 .git/info/refs &&
16
+ test-tool chmtime --get .git/info/refs >a
17
+'
18
+
19
+test_expect_success 'info/refs is not needlessly overwritten' '
20
+ git update-server-info &&
21
+ test-tool chmtime --get .git/info/refs >b &&
22
+ test_cmp a b
23
+'
24
+
25
+test_expect_success 'info/refs can be forced to update' '
26
+ git update-server-info -f &&
27
+ test-tool chmtime --get .git/info/refs >b &&
28
+ ! test_cmp a b
29
+'
30
+
31
+test_expect_success 'info/refs updates when changes are made' '
32
+ test-tool chmtime =0 .git/info/refs &&
33
+ test-tool chmtime --get .git/info/refs >b &&
34
+ test_cmp a b &&
35
+ git update-ref refs/heads/foo HEAD &&
36
+ git update-server-info &&
37
+ test-tool chmtime --get .git/info/refs >b &&
38
+ ! test_cmp a b
39
+'
40
+
41
+test_done