From patchwork Sat May 11 01:34:55 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Wong X-Patchwork-Id: 10939691 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 2950776 for ; Sat, 11 May 2019 01:35:01 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 16C4E1FFBE for ; Sat, 11 May 2019 01:35:01 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 094602012F; Sat, 11 May 2019 01:35:01 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 764861FFBE for ; Sat, 11 May 2019 01:35:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728320AbfEKBe5 (ORCPT ); Fri, 10 May 2019 21:34:57 -0400 Received: from dcvr.yhbt.net ([64.71.152.64]:49650 "EHLO dcvr.yhbt.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728165AbfEKBe5 (ORCPT ); Fri, 10 May 2019 21:34:57 -0400 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 173B51F45F for ; Sat, 11 May 2019 01:34:56 +0000 (UTC) From: Eric Wong To: git@vger.kernel.org Subject: [PATCH] update-server-info: avoid needless overwrites Date: Sat, 11 May 2019 01:34:55 +0000 Message-Id: <20190511013455.5886-1-e@80x24.org> MIME-Version: 1.0 Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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. Signed-off-by: Eric Wong --- server-info.c | 61 +++++++++++++++++++++++++++++------ t/t5200-update-server-info.sh | 41 +++++++++++++++++++++++ 2 files changed, 92 insertions(+), 10 deletions(-) create mode 100755 t/t5200-update-server-info.sh diff --git a/server-info.c b/server-info.c index 41274d098b..34599e4817 100644 --- a/server-info.c +++ b/server-info.c @@ -6,37 +6,78 @@ #include "tag.h" #include "packfile.h" #include "object-store.h" +#include "strbuf.h" + +static int files_differ(FILE *fp, const char *path) +{ + struct stat st; + git_hash_ctx c; + struct object_id oid_old, oid_new; + struct strbuf tmp = STRBUF_INIT; + long new_len = ftell(fp); + + if (new_len < 0 || stat(path, &st) < 0) + return 1; + if (!S_ISREG(st.st_mode)) + return 1; + if ((off_t)new_len != st.st_size) + return 1; + + rewind(fp); + if (strbuf_fread(&tmp, (size_t)new_len, fp) != (size_t)new_len) + return 1; + the_hash_algo->init_fn(&c); + the_hash_algo->update_fn(&c, tmp.buf, tmp.len); + the_hash_algo->final_fn(oid_new.hash, &c); + strbuf_release(&tmp); + + if (strbuf_read_file(&tmp, path, (size_t)st.st_size) < 0) + return 1; + the_hash_algo->init_fn(&c); + the_hash_algo->update_fn(&c, tmp.buf, tmp.len); + the_hash_algo->final_fn(oid_old.hash, &c); + strbuf_release(&tmp); + + return hashcmp(oid_old.hash, oid_new.hash); +} /* * Create the file "path" by writing to a temporary file and renaming * it into place. The contents of the file come from "generate", which * should return non-zero if it encounters an error. */ -static int update_info_file(char *path, int (*generate)(FILE *)) +static int update_info_file(char *path, int (*generate)(FILE *), int force) { char *tmp = mkpathdup("%s_XXXXXX", path); int ret = -1; int fd = -1; FILE *fp = NULL, *to_close; + int do_update; safe_create_leading_directories(path); fd = git_mkstemp_mode(tmp, 0666); if (fd < 0) goto out; - to_close = fp = fdopen(fd, "w"); + to_close = fp = fdopen(fd, "w+"); if (!fp) goto out; fd = -1; ret = generate(fp); if (ret) goto out; + + do_update = force || files_differ(fp, path); fp = NULL; if (fclose(to_close)) goto out; - if (adjust_shared_perm(tmp) < 0) - goto out; - if (rename(tmp, path) < 0) - goto out; + if (do_update) { + if (adjust_shared_perm(tmp) < 0) + goto out; + if (rename(tmp, path) < 0) + goto out; + } else { + unlink(tmp); + } ret = 0; out: @@ -78,10 +119,10 @@ static int generate_info_refs(FILE *fp) return for_each_ref(add_info_ref, fp); } -static int update_info_refs(void) +static int update_info_refs(int force) { char *path = git_pathdup("info/refs"); - int ret = update_info_file(path, generate_info_refs); + int ret = update_info_file(path, generate_info_refs, force); free(path); return ret; } @@ -254,7 +295,7 @@ static int update_info_packs(int force) int ret; init_pack_info(infofile, force); - ret = update_info_file(infofile, write_pack_info_file); + ret = update_info_file(infofile, write_pack_info_file, force); free_pack_info(); free(infofile); return ret; @@ -269,7 +310,7 @@ int update_server_info(int force) */ int errs = 0; - errs = errs | update_info_refs(); + errs = errs | update_info_refs(force); errs = errs | update_info_packs(force); /* remove leftover rev-cache file if there is any */ diff --git a/t/t5200-update-server-info.sh b/t/t5200-update-server-info.sh new file mode 100755 index 0000000000..f1cec46914 --- /dev/null +++ b/t/t5200-update-server-info.sh @@ -0,0 +1,41 @@ +#!/bin/sh + +test_description='Test git stash show configuration.' + +. ./test-lib.sh + +test_expect_success 'setup' 'test_commit file' + +test_expect_success 'create info/refs' ' + git update-server-info && + test_path_is_file .git/info/refs +' + +test_expect_success 'modify and store mtime' ' + test-tool chmtime =0 .git/info/refs && + test-tool chmtime --get .git/info/refs >a +' + +test_expect_success 'info/refs is not needlessly overwritten' ' + git update-server-info && + test-tool chmtime --get .git/info/refs >b && + test_cmp a b +' + +test_expect_success 'info/refs can be forced to update' ' + git update-server-info -f && + test-tool chmtime --get .git/info/refs >b && + ! test_cmp a b +' + +test_expect_success 'info/refs updates when changes are made' ' + test-tool chmtime =0 .git/info/refs && + test-tool chmtime --get .git/info/refs >b && + test_cmp a b && + git update-ref refs/heads/foo HEAD && + git update-server-info && + test-tool chmtime --get .git/info/refs >b && + ! test_cmp a b +' + +test_done From patchwork Wed May 15 00:45:51 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Eric Wong X-Patchwork-Id: 10944157 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 9C29D1390 for ; Wed, 15 May 2019 00:45:55 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7A0F328978 for ; Wed, 15 May 2019 00:45:55 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6764C2898B; Wed, 15 May 2019 00:45:55 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E120228978 for ; Wed, 15 May 2019 00:45:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726195AbfEOApx (ORCPT ); Tue, 14 May 2019 20:45:53 -0400 Received: from dcvr.yhbt.net ([64.71.152.64]:51616 "EHLO dcvr.yhbt.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726044AbfEOApx (ORCPT ); Tue, 14 May 2019 20:45:53 -0400 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 0F1421F461; Wed, 15 May 2019 00:45:52 +0000 (UTC) Date: Wed, 15 May 2019 00:45:51 +0000 From: Eric Wong To: =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason Cc: Jeff King , git@vger.kernel.org, Junio C Hamano Subject: [PATCH 2/1] server-info: conditionally update on fetch Message-ID: <20190515004551.emrxvboqemwnqh4g@dcvr> References: <20190511013455.5886-1-e@80x24.org> <87v9ygwoj0.fsf@evledraar.gmail.com> <20190512040825.GA25370@sigill.intra.peff.net> <87tve0w3ao.fsf@evledraar.gmail.com> <20190514094729.GA12256@sigill.intra.peff.net> <20190514115047.oncvfq24fhnp64re@dcvr> <87ftphw7mv.fsf@evledraar.gmail.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <87ftphw7mv.fsf@evledraar.gmail.com> Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Ævar Arnfjörð Bjarmason wrote: > Aside from this change, I wonder if making "fetch" optionally "exit 1" > if no refs were updated would be useful, as in the below WIP. Of course > it would be better to distinguish errors from "no refs to update". Yes, we should've had this feature all along :) And it's easy for me to build off your WIP to have fetch update server info iff info/refs already exists: -------8<------- Subject: [PATCH 2/1] server-info: conditionally update on fetch Since fetch can invalidate existing server info files, use the new `updated_refs' counter to update server info files iff info/refs already exists. Note: this depends on Ævar's WIP in: https://public-inbox.org/git/87ftphw7mv.fsf@evledraar.gmail.com/ Signed-off-by: Eric Wong --- builtin/fetch.c | 3 +++ server-info.c | 22 +++++++++++++++++++--- t/t5513-fetch-track.sh | 21 +++++++++++++++++++++ 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index da5414d9db..b35d4d105d 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1681,6 +1681,9 @@ int cmd_fetch(int argc, const char **argv, const char *prefix) close_all_packs(the_repository->objects); + if (updated_refs) + update_server_info(-1); + argv_array_pushl(&argv_gc_auto, "gc", "--auto", NULL); if (verbosity < 0) argv_array_push(&argv_gc_auto, "--quiet"); diff --git a/server-info.c b/server-info.c index e68f785c2f..d4065d56a3 100644 --- a/server-info.c +++ b/server-info.c @@ -7,6 +7,7 @@ #include "packfile.h" #include "object-store.h" #include "strbuf.h" +#include "dir.h" struct update_info_ctx { FILE *cur_fp; @@ -170,10 +171,25 @@ static int generate_info_refs(struct update_info_ctx *uic) return for_each_ref(add_info_ref, uic); } -static int update_info_refs(int force) +static int want_update(int *force, const char *path) +{ + if (*force < 0) { + if (file_exists(path)) + *force = 0; /* continue to normal update */ + else + return 0; + } + return 1; +} + +static int update_info_refs(int *force) { char *path = git_pathdup("info/refs"); - int ret = update_info_file(path, generate_info_refs, force); + int ret = 0; + + if (want_update(force, path)) + ret = update_info_file(path, generate_info_refs, *force); + free(path); return ret; } @@ -361,7 +377,7 @@ int update_server_info(int force) */ int errs = 0; - errs = errs | update_info_refs(force); + errs = errs | update_info_refs(&force); errs = errs | update_info_packs(force); /* remove leftover rev-cache file if there is any */ diff --git a/t/t5513-fetch-track.sh b/t/t5513-fetch-track.sh index 65d1e05bd6..421f16ddfd 100755 --- a/t/t5513-fetch-track.sh +++ b/t/t5513-fetch-track.sh @@ -27,4 +27,25 @@ test_expect_success fetch ' ) ' +test_expect_success 'info/refs not created by fetch' ' + ( + cd other && + test_path_is_dir .git/info && + ! test_path_is_file .git/info/refs + ) +' + +test_expect_success 'info/refs updated by fetch if it already exists' ' + git branch b/for-info-refs && + ( + cd other && + git update-server-info && + test_path_is_file .git/info/refs && + ! grep b/for-info-refs .git/info/refs && + git fetch && + test_path_is_file .git/info/refs && + grep b/for-info-refs .git/info/refs + ) +' + test_done