[PATCH] Display the most recent modtime for the repolist

[PATCH] Display the most recent modtime for the repolist

[fparent]

Instead of using the modtime of the master branch cgit now uses the modtime
of the lastest ref updated.
---
 ui-repolist.c | 32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/ui-repolist.c b/ui-repolist.c
index 2ab6e9e..dcd74dc 100644
--- a/ui-repolist.c
+++ b/ui-repolist.c
@@ -13,6 +13,11 @@
 #include "ui-shared.h"
 #include <strings.h>
 
+struct ref_mod_time {
+	struct cgit_repo *repo;
+	time_t mod_time;
+};
+
 static time_t read_agefile(char *path)
 {
 	time_t result;
@@ -74,11 +79,32 @@ end:
 	return (r->mtime != 0);
 }
 
+static int get_ref_mod_time(const char *refname, const unsigned char *sha1,
+			    int flags, void *cb_data)
+{
+	struct ref_mod_time *rmt = (struct ref_mod_time*) cb_data;
+	char *defbranch_old;
+
+	defbranch_old = rmt->repo->defbranch;
+	rmt->repo->defbranch = (char*) refname;
+	get_repo_modtime(rmt->repo, &rmt->mod_time);
+	rmt->repo->defbranch = defbranch_old;
+	return 0;
+}
+
 static void print_modtime(struct cgit_repo *repo)
 {
-	time_t t;
-	if (get_repo_modtime(repo, &t))
-		cgit_print_age(t, -1, NULL);
+	struct ref_mod_time rmt = { .repo = repo };
+	struct ref_mod_time last_mod_ref = {0};
+
+	/* Get the time of the most recent update to the repo */
+	setenv("GIT_DIR", repo->path, 1);
+	for_each_branch_ref(get_ref_mod_time, &rmt);
+	if (rmt.mod_time > last_mod_ref.mod_time)
+		last_mod_ref = rmt;
+
+	if (last_mod_ref.mod_time)
+		cgit_print_age(last_mod_ref.mod_time, -1, NULL);
 }
 
 static int is_match(struct cgit_repo *repo)
-- 
1.8.4.rc1

[PATCH] Display the most recent modtime for the repolist

[mailings]

This should be done with the agefile.

This has come up before.

NAK

On 22/08/13 21:05, Fabien Parent wrote:
> Instead of using the modtime of the master branch cgit now uses the modtime
> of the lastest ref updated.
> ---
>  ui-repolist.c | 32 +++++++++++++++++++++++++++++---
>  1 file changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/ui-repolist.c b/ui-repolist.c
> index 2ab6e9e..dcd74dc 100644
> --- a/ui-repolist.c
> +++ b/ui-repolist.c
> @@ -13,6 +13,11 @@
>  #include "ui-shared.h"
>  #include <strings.h>
>  
> +struct ref_mod_time {
> +	struct cgit_repo *repo;
> +	time_t mod_time;
> +};
> +
>  static time_t read_agefile(char *path)
>  {
>  	time_t result;
> @@ -74,11 +79,32 @@ end:
>  	return (r->mtime != 0);
>  }
>  
> +static int get_ref_mod_time(const char *refname, const unsigned char *sha1,
> +			    int flags, void *cb_data)
> +{
> +	struct ref_mod_time *rmt = (struct ref_mod_time*) cb_data;
> +	char *defbranch_old;
> +
> +	defbranch_old = rmt->repo->defbranch;
> +	rmt->repo->defbranch = (char*) refname;
> +	get_repo_modtime(rmt->repo, &rmt->mod_time);
> +	rmt->repo->defbranch = defbranch_old;
> +	return 0;
> +}
> +
>  static void print_modtime(struct cgit_repo *repo)
>  {
> -	time_t t;
> -	if (get_repo_modtime(repo, &t))
> -		cgit_print_age(t, -1, NULL);
> +	struct ref_mod_time rmt = { .repo = repo };
> +	struct ref_mod_time last_mod_ref = {0};
> +
> +	/* Get the time of the most recent update to the repo */
> +	setenv("GIT_DIR", repo->path, 1);
> +	for_each_branch_ref(get_ref_mod_time, &rmt);
> +	if (rmt.mod_time > last_mod_ref.mod_time)
> +		last_mod_ref = rmt;
> +
> +	if (last_mod_ref.mod_time)
> +		cgit_print_age(last_mod_ref.mod_time, -1, NULL);
>  }
>  
>  static int is_match(struct cgit_repo *repo)
> 

-- 
Ferry Huberts

[PATCH] Display the most recent modtime for the repolist

[john]

On Thu, Aug 22, 2013 at 09:05:33PM +0200, Fabien Parent wrote:
> Instead of using the modtime of the master branch cgit now uses the modtime
> of the lastest ref updated.
> ---
>  ui-repolist.c | 32 +++++++++++++++++++++++++++++---
>  1 file changed, 29 insertions(+), 3 deletions(-)
> 
>
> diff --git a/ui-repolist.c b/ui-repolist.c
> index 2ab6e9e..dcd74dc 100644
> --- a/ui-repolist.c
> +++ b/ui-repolist.c
> @@ -13,6 +13,11 @@
>  #include "ui-shared.h"
>  #include <strings.h>
>  
> +struct ref_mod_time {
> +	struct cgit_repo *repo;
> +	time_t mod_time;
> +};
> +
>  static time_t read_agefile(char *path)
>  {
>  	time_t result;
> @@ -74,11 +79,32 @@ end:
>  	return (r->mtime != 0);
>  }
>  
> +static int get_ref_mod_time(const char *refname, const unsigned char *sha1,
> +			    int flags, void *cb_data)
> +{
> +	struct ref_mod_time *rmt = (struct ref_mod_time*) cb_data;
> +	char *defbranch_old;
> +
> +	defbranch_old = rmt->repo->defbranch;
> +	rmt->repo->defbranch = (char*) refname;
> +	get_repo_modtime(rmt->repo, &rmt->mod_time);

By doing this for each ref, don't we end up with repo->modtime pointing
at which ever branch is processed last?  There doesn't seem to be an
attempt to update it below.

Also this doesn't change the behaviour to what people normally want - by
sticking with get_repo_modtime you're still just stat'ing the ref file
(assuming it's not packed).  For repositories that spend a long time
idle, a GC or other operation may well change the mtime of the file
without any actual change to the repository.

> +	rmt->repo->defbranch = defbranch_old;
> +	return 0;
> +}
> +
>  static void print_modtime(struct cgit_repo *repo)
>  {
> -	time_t t;
> -	if (get_repo_modtime(repo, &t))
> -		cgit_print_age(t, -1, NULL);
> +	struct ref_mod_time rmt = { .repo = repo };
> +	struct ref_mod_time last_mod_ref = {0};
> +
> +	/* Get the time of the most recent update to the repo */
> +	setenv("GIT_DIR", repo->path, 1);
> +	for_each_branch_ref(get_ref_mod_time, &rmt);

So we unconditionally examine ever ref here?  When a user's configured
an age file that will return the same value every time -
get_repo_modtime will never actually inspect the branch.

Also, by putting the change only in print_modtime this is going to
become inconsistent with the sort order when get_repo_modtime is used.

If we want to do a change like this, then it should be contained in
get_repo_modtime (probably with a new helper function, but it shouldn't
need to touch any other functions here).  We probably also want it
controlled by a config variable to let users with a lot of repositories
and refs avoid paying the cost of examining all of them.

> +	if (rmt.mod_time > last_mod_ref.mod_time)
> +		last_mod_ref = rmt;
> +
> +	if (last_mod_ref.mod_time)
> +		cgit_print_age(last_mod_ref.mod_time, -1, NULL);
>  }
>  
>  static int is_match(struct cgit_repo *repo)
> -- 
> 1.8.4.rc1

[PATCH v2] ui-repolist: use latest branch modtime as Idle

[fparent]

Instead of using the modtime of the master branch cgit now uses the modtime
of the latest updated branch. This option is disabled by default to avoid
performance hit on repository with many branches.
---
 cgit.c        |  3 +++
 cgit.h        |  1 +
 ui-repolist.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++-----------
 3 files changed, 65 insertions(+), 14 deletions(-)

diff --git a/cgit.c b/cgit.c
index 861352a..9d49f88 100644
--- a/cgit.c
+++ b/cgit.c
@@ -291,6 +291,8 @@ static void config_cb(const char *name, const char *value)
 		add_mimetype(name + 9, value);
 	else if (!strcmp(name, "include"))
 		parse_configfile(expand_macros(value), config_cb);
+	else if (!strcmp(name, "enable-branch-mod-time"))
+		ctx.cfg.enable_branch_mod_time = atoi(value);
 }
 
 static void querystring_cb(const char *name, const char *value)
@@ -351,6 +353,7 @@ static void prepare_context(struct cgit_context *ctx)
 {
 	memset(ctx, 0, sizeof(*ctx));
 	ctx->cfg.agefile = "info/web/last-modified";
+	ctx->cfg.enable_branch_mod_time = 0;
 	ctx->cfg.nocache = 0;
 	ctx->cfg.cache_size = 0;
 	ctx->cfg.cache_max_create_time = 5;
diff --git a/cgit.h b/cgit.h
index a474d77..ec12aad 100644
--- a/cgit.h
+++ b/cgit.h
@@ -238,6 +238,7 @@ struct cgit_config {
 	int ssdiff;
 	int branch_sort;
 	int commit_sort;
+	int enable_branch_mod_time;
 	struct string_list mimetypes;
 	struct cgit_filter *about_filter;
 	struct cgit_filter *commit_filter;
diff --git a/ui-repolist.c b/ui-repolist.c
index 2ab6e9e..78c1f45 100644
--- a/ui-repolist.c
+++ b/ui-repolist.c
@@ -13,6 +13,11 @@
 #include "ui-shared.h"
 #include <strings.h>
 
+struct branch_mod_time {
+	const struct cgit_repo *repo;
+	time_t mod_time;
+};
+
 static time_t read_agefile(char *path)
 {
 	time_t result;
@@ -31,11 +36,44 @@ static time_t read_agefile(char *path)
 	return result;
 }
 
+static int get_file_modtime(const char *const path, time_t *mtime)
+{
+	struct stat s;
+
+	if (stat(path, &s) == 0) {
+		*mtime = s.st_mtime;
+		return 1;
+	}
+
+	return 0;
+}
+
+static int get_branch_mod_time(const char *refname, const unsigned char *sha1,
+			       int flags, void *cb_data)
+{
+	time_t t;
+	struct branch_mod_time *rmt = (struct branch_mod_time*) cb_data;
+
+	if (get_file_modtime(sha1_file_name(sha1), &t)) {
+		rmt->mod_time = t > rmt->mod_time ? t : rmt->mod_time;
+		goto end;
+	}
+
+	if (get_file_modtime(sha1_pack_name(sha1), &t)) {
+		rmt->mod_time = t > rmt->mod_time ? t : rmt->mod_time;
+		goto end;
+	}
+
+end:
+	return 0;
+}
+
 static int get_repo_modtime(const struct cgit_repo *repo, time_t *mtime)
 {
 	struct strbuf path = STRBUF_INIT;
 	struct stat s;
 	struct cgit_repo *r = (struct cgit_repo *)repo;
+	struct branch_mod_time rmt = { .repo = repo };
 
 	if (repo->mtime != -1) {
 		*mtime = repo->mtime;
@@ -50,21 +88,30 @@ static int get_repo_modtime(const struct cgit_repo *repo, time_t *mtime)
 		}
 	}
 
-	strbuf_reset(&path);
-	strbuf_addf(&path, "%s/refs/heads/%s", repo->path,
-		    repo->defbranch ? repo->defbranch : "master");
-	if (stat(path.buf, &s) == 0) {
-		*mtime = s.st_mtime;
-		r->mtime = *mtime;
-		goto end;
-	}
+	if (ctx.cfg.enable_branch_mod_time) {
+		/* Get the time of the most recent update to the repo */
+		setenv("GIT_DIR", repo->path, 1);
+		for_each_branch_ref(get_branch_mod_time, &rmt);
+		if (rmt.mod_time) {
+			*mtime = rmt.mod_time;
+			r->mtime = *mtime;
+			goto end;
+		}
+	} else {
+		strbuf_reset(&path);
+		strbuf_addf(&path, "%s/refs/heads/%s", repo->path,
+			    repo->defbranch ? repo->defbranch : "master");
+		if (get_file_modtime(path.buf, mtime)) {
+			r->mtime = *mtime;
+			goto end;
+		}
 
-	strbuf_reset(&path);
-	strbuf_addf(&path, "%s/%s", repo->path, "packed-refs");
-	if (stat(path.buf, &s) == 0) {
-		*mtime = s.st_mtime;
-		r->mtime = *mtime;
-		goto end;
+		strbuf_reset(&path);
+		strbuf_addf(&path, "%s/%s", repo->path, "packed-refs");
+		if (get_file_modtime(path.buf, mtime)) {
+			r->mtime = *mtime;
+			goto end;
+		}
 	}
 
 	*mtime = 0;
-- 
1.8.4.rc3