From 9934c3f211bca05a79ebf3d6a64fc8a46e977fc6 Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Mon, 15 Jun 2026 16:10:12 +0100 Subject: [PATCH 01/12] Port XML parser to expat The handwritten parser didn't handle tags with - characters in them and since the full set of accepted characters includes unicode and full unicode support would be awkward since libc doesn't directly expose utf-8/utf-16 encoding support it makes sense to let a battle-tested implementation deal with it. --- Makefile | 2 +- include/xml.h | 3 +- src/manifest.c | 8 +- src/xml.c | 365 +++++++++++++++++++------------------------------ 4 files changed, 151 insertions(+), 227 deletions(-) diff --git a/Makefile b/Makefile index 66c5bd7..f92c6e6 100644 --- a/Makefile +++ b/Makefile @@ -5,7 +5,7 @@ INCLUDE = -I include CFLAGS_COMMON = $(INCLUDE) -Wall -Wextra -Werror -MD -MP CFLAGS_RELEASE = -DNDEBUG -O3 $(CFLAGS_COMMON) CFLAGS_DEBUG = -O0 -g $(CFLAGS_COMMON) -LDFLAGS_DEBUG = -lm +LDFLAGS_DEBUG = -lm -lexpat LDFLAGS_RELEASE = -s $(LDFLAGS_DEBUG) SRC = $(shell find src -type f) diff --git a/include/xml.h b/include/xml.h index 146c8bb..04f4222 100644 --- a/include/xml.h +++ b/include/xml.h @@ -18,6 +18,7 @@ #ifndef __xml_h__ #define __xml_h__ +#include typedef struct xml_tag_s xml_tag_t; typedef struct xml_field_s xml_field_t; @@ -40,7 +41,7 @@ struct xml_tag_s -extern xml_tag_t* xml_document_parse(const char* source); +extern xml_tag_t* xml_document_parse(const char* source, size_t len); extern const char* xml_tag_field(xml_tag_t* tag, const char* name); extern void xml_tag_delete(xml_tag_t* tag); diff --git a/src/manifest.c b/src/manifest.c index 91a4a91..4ae4365 100644 --- a/src/manifest.c +++ b/src/manifest.c @@ -299,6 +299,12 @@ manifest_t* manifest_read(const char* path) return NULL; } + if (manifest_stat.st_size < 0) + { + close(fd); + fprintf(stderr, "Error: File %s has negative size.\n", path); + } + char manifest_string[manifest_stat.st_size + 1]; if (read(fd, manifest_string, manifest_stat.st_size) < 0) { @@ -311,7 +317,7 @@ manifest_t* manifest_read(const char* path) close(fd); xml_tag_t* manifest_xml - = xml_document_parse(manifest_string); + = xml_document_parse(manifest_string, manifest_stat.st_size); if (!manifest_xml) { fprintf(stderr, "Error: Failed to parse xml in manifest file.\n"); diff --git a/src/xml.c b/src/xml.c index 55298c7..4a78cfb 100644 --- a/src/xml.c +++ b/src/xml.c @@ -23,7 +23,11 @@ #include #include #include +#include +#include +#include +#include xml_tag_t* xml__tag_create(const char* name, xml_tag_t* parent); @@ -32,102 +36,6 @@ bool xml__tag_insert_tag(xml_tag_t* tag, xml_tag_t* child); -unsigned xml__parse_comment(const char* source) -{ - if (!source) - return 0; - if (strncmp(source, "", 3) == 0) - { - i += 3; - break; - } - } - - return i; -} - -unsigned xml__parse_whitespace(const char* source) -{ - if (!source) - return 0; - - unsigned i; - for (i = 0; isspace(source[i]); i++); - - unsigned c = xml__parse_comment(&source[i]); - if (c) - { - i += c; - i += xml__parse_whitespace(&source[i]); - } - - return i; -} - -unsigned xml__parse_field(const char* source, xml_field_t** field) -{ - if (!source) - return 0; - - unsigned i = 0; - i += xml__parse_whitespace(&source[i]); - - const char* name = &source[i]; - if (!isalpha(source[i]) - && (source[i] != '_')) - return 0; - unsigned n; - for (n = 1; isalnum(source[i + n]) - || (source[i + n] == '_') - || (source[i + n] == '-') - ; n++); - i += n; - - if (source[i++] != '=') - return 0; - - if (source[i++] != '\"') - return 0; - const char* data = &source[i]; - unsigned d; - for (d = 0; (source[i + d] != '\"') - && (source[i + d] != '\0'); d++); - i += d; - if (source[i++] != '\"') - return 0; - - i += xml__parse_whitespace(&source[i]); - - if (field) - { - xml_field_t* nfield - = (xml_field_t*)malloc(sizeof(xml_field_t) - + n + d + 2); - if (!nfield) return 0; - - nfield->name = (char*)((uintptr_t)nfield + sizeof(xml_field_t)); - nfield->data = (char*)((uintptr_t)nfield->name + n + 1); - - memcpy(nfield->name, name, n); - nfield->name[n] = '\0'; - - memcpy(nfield->data, data, d); - nfield->data[d] = '\0'; - - *field = nfield; - } - - return i; -} - - - xml_tag_t* xml__tag_create(const char* name, xml_tag_t* parent) { xml_tag_t* tag @@ -175,6 +83,28 @@ void xml_tag_delete(xml_tag_t* tag) free(tag); } +xml_field_t* xml__field_create(const char* name, const char* data) +{ + size_t n = strlen(name); + size_t d = strlen(data); + xml_field_t* nfield + = (xml_field_t*)malloc(sizeof(xml_field_t) + + n + d + 2); + if (!nfield) return NULL; + + nfield->name = (char*)((uintptr_t)nfield + sizeof(xml_field_t)); + nfield->data = (char*)((uintptr_t)nfield->name + n + 1); + + memcpy(nfield->name, name, n); + nfield->name[n] = '\0'; + + memcpy(nfield->data, data, d); + nfield->data[d] = '\0'; + + return nfield; +} + + bool xml__tag_append_field(xml_tag_t* tag, xml_field_t* field) { if (!tag || !field) @@ -206,166 +136,153 @@ bool xml__tag_insert_tag(xml_tag_t* tag, xml_tag_t* child) return true; } - - -unsigned xml__parse_tag(const char* source, xml_tag_t** tag) +typedef enum { - if (!source) - return 0; - - unsigned i = 0; - i += xml__parse_whitespace(&source[i]); - - if (source[i++] != '<') - return 0; - - const char* name = &source[i]; - if (!isalpha(source[i]) - && (source[i] != '_')) - return 0; - unsigned n; - for (n = 1; isalnum(source[i + n]) || (source[i + n] == '_'); n++); - i += n; - - i += xml__parse_whitespace(&source[i]); - - char nstr[n + 1]; - memcpy(nstr, name, n); - nstr[n] = '\0'; - - xml_tag_t* ntag = xml__tag_create(nstr, NULL); - if (!ntag) return 0; - - while (true) - { - xml_field_t* field; - unsigned field_length = xml__parse_field(&source[i], &field); - if (field_length == 0) break; - - if (!xml__tag_append_field(ntag, field)) - { - free(field); - xml_tag_delete(ntag); - return 0; - } - - i += field_length; + xml__RESULT_SUCCESS, + xml__RESULT_CREATE_TAG_FAILED, + xml__RESULT_INSERT_TAG_FAILED, + xml__RESULT_CREATE_FIELD_FAILED, + xml__RESULT_APPEND_FIELD_FAILED, +} xml__result_e; + +const char* xml__result_string(xml__result_e e) +{ + switch (e) { + case xml__RESULT_SUCCESS: + return "Succeded"; + case xml__RESULT_CREATE_TAG_FAILED: + return "Creating tag value failed"; + case xml__RESULT_INSERT_TAG_FAILED: + return "Inserting tag into parent failed"; + case xml__RESULT_CREATE_FIELD_FAILED: + return "Creating field value failed"; + case xml__RESULT_APPEND_FIELD_FAILED: + return "Appending field into tag failed"; + default: + return "INVAILD RESULT VALUE!"; } +} - i += xml__parse_whitespace(&source[i]); +typedef struct xml__state_s +{ + XML_Parser parser; + xml_tag_t* current; + xml__result_e result; +} xml__state_t; - bool empty = (source[i] == '/'); - if (empty) i++; +void xml__stop_parser(xml__state_t* state, xml__result_e code) +{ + enum XML_Status res = XML_StopParser(state->parser, XML_FALSE); + // We should only stop once, + // so shouldn't fail from already having stopped. + assert(res == XML_STATUS_OK); + (void)res; + state->result = code; +} - if (source[i++] != '>') +static void XMLCALL xml__start_element(void* ud, const XML_Char* name, const XML_Char** attrs) +{ +#ifdef XML_UNICODE +#error "Conversion from wide characters to multibyte streams is not implemented" +#else + xml__state_t* state = (xml__state_t*)ud; + xml_tag_t* ntag = xml__tag_create(name, state->current); + if (!ntag) { - xml_tag_delete(ntag); - return 0; + xml__stop_parser(state, xml__RESULT_CREATE_TAG_FAILED); + return; } - i += xml__parse_whitespace(&source[i]); - if (!empty) + while (attrs[0] != NULL) { - while (true) + xml_field_t *field = xml__field_create(attrs[0], attrs[1]); + if (!field) { - xml_tag_t* ctag; - unsigned ctag_length = xml__parse_tag(&source[i], &ctag); - if (ctag_length == 0) break; - - if (!xml__tag_insert_tag(ntag, ctag)) - { - xml_tag_delete(ctag); - xml_tag_delete(ntag); - return 0; - } - - i += ctag_length; + xml_tag_delete(ntag); + xml__stop_parser(state, xml__RESULT_CREATE_FIELD_FAILED); + return; } - i += xml__parse_whitespace(&source[i]); - if ((strncmp(&source[i], "')) + if (!xml__tag_append_field(ntag, field)) { + free(field); xml_tag_delete(ntag); - return 0; + xml__stop_parser(state, xml__RESULT_APPEND_FIELD_FAILED); + return; } - i += (2 + n + 1); - i += xml__parse_whitespace(&source[i]); + attrs += 2; } - if (tag) - *tag = ntag; - else - xml_tag_delete(ntag); - - return i; + state->current = ntag; +#endif } - - -xml_tag_t* xml_document_parse(const char* source) +static void XMLCALL xml__end_element(void* ud, const XML_Char* /*name*/) { - xml_tag_t* document - = xml__tag_create(NULL, NULL); - if (!document) return NULL; - - unsigned i = 0; - i += xml__parse_whitespace(&source[i]); - - if (strncmp(&source[i], "result != xml__RESULT_SUCCESS) { - i += 5; - - while (true) - { - xml_field_t* field; - unsigned field_length = xml__parse_field(&source[i], &field); - if (field_length == 0) break; - - if (!xml__tag_append_field(document, field)) - { - free(field); - xml_tag_delete(document); - return NULL; - } - - i += field_length; - } - - i += xml__parse_whitespace(&source[i]); - if (strncmp(&source[i], "?>", 2) != 0) - { - xml_tag_delete(document); - return NULL; - } - i += 2; - i += xml__parse_whitespace(&source[i]); + // We assume if an error happened in an empty element start + // that the current element was never updated so we don't + // need to reset it. + return; } - while (true) + // finish the current tag by inserting it into the parent tag + xml_tag_t *current = state->current; + xml_tag_t *parent = current->parent; + if (!xml__tag_insert_tag(parent, current)) { - xml_tag_t* tag; - unsigned tag_length = xml__parse_tag(&source[i], &tag); - if (tag_length == 0) break; + xml_tag_delete(current); + xml__stop_parser(state, xml__RESULT_INSERT_TAG_FAILED); + // Intentional fallthrough + } + state->current = parent; +} - if (!xml__tag_insert_tag(document, tag)) - { - xml_tag_delete(tag); - xml_tag_delete(document); - return NULL; - } +xml_tag_t* xml_document_parse(const char* source, size_t len) +{ + if (len > INT_MAX) { + fprintf(stderr, "Error: Document size %zu exceeds maximum expat's API can accept.\n", len); + return NULL; + } - i += tag_length; + xml__state_t state; + state.result = xml__RESULT_SUCCESS; + xml_tag_t* document = xml__tag_create(NULL, NULL); + if (!document) { + fprintf(stderr, "Error: Failed to create empty document tag.\n"); + return NULL; } - i += xml__parse_whitespace(&source[i]); + state.current = document; - if (source[i] != '\0') - { + state.parser = XML_ParserCreate(NULL); + if (!state.parser) { + fprintf(stderr, "Error: Failed to create XML parser.\n"); + return NULL; + } + XML_SetElementHandler(state.parser, xml__start_element, xml__end_element); + XML_SetUserData(state.parser, &state); + + enum XML_Status result = XML_Parse(state.parser, source, len, /*isFinal*/ 1); + if (result != XML_STATUS_OK) { + fprintf(stderr, + "Error: Parsing failed with parser status: %s and processor status: %s.\n", + XML_ErrorString(XML_GetErrorCode(state.parser)), + xml__result_string(state.result)); + // Free from the root document instead of the current state + // since delete is recursive and will get the current tag + // but the current tag may not be popped off the stack back + // to the root document tag in the case of error. xml_tag_delete(document); + XML_ParserFree(state.parser); return NULL; } + XML_ParserFree(state.parser); return document; } From 976d564bd04811fc4722e803ad3f00aeadd3116d Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Mon, 15 Jun 2026 17:06:10 +0100 Subject: [PATCH 02/12] Parse linkfiles elements from XML --- include/manifest.h | 2 + src/manifest.c | 144 +++++++++++++++++++++++++++++++++++---------- 2 files changed, 115 insertions(+), 31 deletions(-) diff --git a/include/manifest.h b/include/manifest.h index a47bd3f..093aa73 100644 --- a/include/manifest.h +++ b/include/manifest.h @@ -44,6 +44,8 @@ typedef struct const char* revision; copyfile_t* copyfile; unsigned copyfile_count; + copyfile_t* linkfile; + unsigned linkfile_count; group_t* group; unsigned group_count; } project_t; diff --git a/src/manifest.c b/src/manifest.c index 4ae4365..1dd14f0 100644 --- a/src/manifest.c +++ b/src/manifest.c @@ -40,6 +40,7 @@ void manifest_delete(manifest_t* manifest) for (i = 0; i < manifest->project_count; i++) { free(manifest->project[i].copyfile); + free(manifest->project[i].linkfile); free(manifest->project[i].group); } @@ -203,54 +204,77 @@ manifest_t* manifest_parse(xml_tag_t* document) project->copyfile_count = 0; project->copyfile = NULL; + project->linkfile_count = 0; + project->linkfile = NULL; + project->group_count = 0; project->group = NULL; unsigned k; for (k = 0; k < mdoc->tag[i]->tag_count; k++) { - if (strcmp(mdoc->tag[i]->tag[k]->name, "copyfile") != 0) + const char* tag_name = mdoc->tag[i]->tag[k]->name; + + const char *copyfile_kind; + copyfile_t** copyfile; + unsigned* copyfile_count; + if (strcmp(tag_name, "copyfile") == 0) + { + copyfile_kind = "copyfile"; + copyfile = &project->copyfile; + copyfile_count = &project->copyfile_count; + } + else if (strcmp(tag_name, "linkfile") == 0) + { + copyfile_kind = "linkfile"; + copyfile = &project->linkfile; + copyfile_count = &project->linkfile_count; + } + else { fprintf(stderr, "Warning: Unknown project sub-tag '%s'.\n", - mdoc->tag[i]->tag[k]->name); + tag_name); continue; } copyfile_t* ncopyfile - = (copyfile_t*)realloc(project->copyfile, - (project->copyfile_count + 1) * sizeof(copyfile_t)); + = (copyfile_t*)realloc(*copyfile, + ((*copyfile_count) + 1) * sizeof(copyfile_t)); if (!ncopyfile) { fprintf(stderr, - "Error: Failed to add copyfile to project.\n"); + "Error: Failed to add %s to project.\n", + copyfile_kind); manifest_delete(manifest); return NULL; } - project->copyfile = ncopyfile; - project->copyfile[project->copyfile_count].source + *copyfile = ncopyfile; + (*copyfile)[*copyfile_count].source = xml_tag_field(mdoc->tag[i]->tag[k], "src"); - project->copyfile[project->copyfile_count].dest + (*copyfile)[*copyfile_count].dest = xml_tag_field(mdoc->tag[i]->tag[k], "dest"); - if (!project->copyfile[project->copyfile_count].source) + if (!(*copyfile)[*copyfile_count].source) { fprintf(stderr, - "Error: Invalid copyfile tag, missing source field.\n"); + "Error: Invalid %s tag, missing source field.\n", + copyfile_kind); manifest_delete(manifest); return NULL; } - if (!project->copyfile[project->copyfile_count].dest) + if (!(*copyfile)[*copyfile_count].dest) { fprintf(stderr, - "Error: Invalid copyfile tag, missing dest field.\n"); + "Error: Invalid %s tag, missing dest field.\n", + copyfile_kind); manifest_delete(manifest); return NULL; } - project->copyfile_count++; + (*copyfile_count)++; } const char* groups @@ -336,7 +360,27 @@ manifest_t* manifest_read(const char* path) return manifest; } - +// Create a new copyfiles by copying the source copyfiles. +// On success the new copyfiles is returned +// and the length is stored in dst_copyfile_count. +// On failure NULL is returned and dst_copyfile_count is untouched. +copyfile_t* manifest__copyfiles_copy( + copyfile_t* src_copyfile, + unsigned src_copyfile_count, + unsigned* dst_copyfile_count) +{ + copyfile_t* ncopyfile + = (copyfile_t*)malloc( + src_copyfile_count * sizeof(copyfile_t)); + if (!ncopyfile) return NULL; + + memcpy( + ncopyfile, + src_copyfile, + (src_copyfile_count * sizeof(copyfile_t))); + *dst_copyfile_count = src_copyfile_count; + return ncopyfile; +} manifest_t* manifest_copy(manifest_t* a) { @@ -362,6 +406,8 @@ manifest_t* manifest_copy(manifest_t* a) manifest->project[i] = a->project[i]; manifest->project[i].copyfile = NULL; manifest->project[i].copyfile_count = 0; + manifest->project[i].linkfile = NULL; + manifest->project[i].linkfile_count = 0; manifest->project[i].group = NULL; manifest->project[i].group_count = 0; } @@ -371,19 +417,28 @@ manifest_t* manifest_copy(manifest_t* a) if (a->project[i].copyfile_count) { manifest->project[i].copyfile - = (copyfile_t*)malloc( - a->project[i].copyfile_count * sizeof(copyfile_t)); + = manifest__copyfiles_copy( + a->project[i].copyfile, + a->project[i].copyfile_count, + &manifest->project[i].copyfile_count); if (!manifest->project[i].copyfile) { manifest_delete(manifest); return NULL; } - memcpy( - manifest->project[i].copyfile, - a->project[i].copyfile, - (a->project[i].copyfile_count * sizeof(copyfile_t))); - manifest->project[i].copyfile_count - = a->project[i].copyfile_count; + } + if (a->project[i].linkfile_count) + { + manifest->project[i].linkfile + = manifest__copyfiles_copy( + a->project[i].linkfile, + a->project[i].linkfile_count, + &manifest->project[i].linkfile_count); + if (!manifest->project[i].linkfile) + { + manifest_delete(manifest); + return NULL; + } } if (a->project[i].group_count) { @@ -571,6 +626,24 @@ bool manifest_write_snapshot(manifest_t* manifest, const char* path) { fprintf(fp, "/>\n"); } + + if (project->linkfile_count) + { + fprintf(fp, ">\n"); + + unsigned j; + for (j = 0; j < project->linkfile_count; j++) + { + fprintf(fp, "\t\t\n", + project->linkfile[j].source, project->linkfile[j].dest); + } + + fprintf(fp, "\t\n"); + } + else + { + fprintf(fp, "/>\n"); + } } fprintf(fp, "\n"); @@ -670,21 +743,30 @@ manifest_t* manifest_group_filter( if (manifest->project[i].copyfile_count) { - filtered->project[j].copyfile - = (copyfile_t*)malloc( - manifest->project[i].copyfile_count * sizeof(copyfile_t)); + filtered->project[j].copyfile = manifest__copyfiles_copy( + manifest->project[j].copyfile, + manifest->project[j].copyfile_count, + &filtered->project[j].copyfile_count); if (!filtered->project[j].copyfile) { manifest_delete(filtered); return NULL; } - memcpy( - filtered->project[j].copyfile, - manifest->project[i].copyfile, - (manifest->project[i].copyfile_count * sizeof(copyfile_t))); - filtered->project[j].copyfile_count - = manifest->project[i].copyfile_count; } + + if (manifest->project[i].linkfile_count) + { + filtered->project[j].linkfile = manifest__copyfiles_copy( + manifest->project[j].linkfile, + manifest->project[j].linkfile_count, + &filtered->project[j].linkfile_count); + if (!filtered->project[j].linkfile) + { + manifest_delete(filtered); + return NULL; + } + } + if (manifest->project[i].group_count) { if (!group_list_copy( From 5f385a9ae2981bd747e2c9fb23b25ea0f094fdc0 Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Mon, 15 Jun 2026 17:30:48 +0100 Subject: [PATCH 03/12] Use intermediate variables to simplify attribute paths in sync thread --- src/main.c | 68 ++++++++++++++++++++++++++++-------------------------- 1 file changed, 35 insertions(+), 33 deletions(-) diff --git a/src/main.c b/src/main.c index b7cdff9..a71c1ae 100644 --- a/src/main.c +++ b/src/main.c @@ -79,13 +79,14 @@ static void* frepo_sync_manifest__thread(void* param) = (volatile struct manifest_thread_params*)param; unsigned p = tp->project; + project_t* project = &tp->manifest->project[p]; - bool exists = git_exists(tp->manifest->project[p].path); + bool exists = git_exists(project->path); printf("%s repository (%u/%u) '%s'.\n", (exists ? "Updating" : "Cloning"), (p + 1), tp->manifest->project_count, - tp->manifest->project[p].path); + project->path); char* revision = NULL; bool revision_differs = false; @@ -93,26 +94,26 @@ static void* frepo_sync_manifest__thread(void* param) if (exists && !tp->mirror) { revision = git_current_branch( - tp->manifest->project[p].path); + project->path); if (!revision) { fprintf(stderr, "Error: Failed to check current revision of '%s'.\n", - tp->manifest->project[p].path); + project->path); *(tp->error) = true; sem_post(tp->semaphore); return NULL; } revision_differs - = (strcmp(revision, tp->manifest->project[p].revision) != 0); + = (strcmp(revision, project->revision) != 0); if (revision_differs && !git_checkout( - tp->manifest->project[p].path, - tp->manifest->project[p].revision, false)) + project->path, + project->revision, false)) { free(revision); fprintf(stderr, "Error: Failed to checkout revision '%s' of '%s'.\n", - tp->manifest->project[p].revision, - tp->manifest->project[p].path); + project->revision, + project->path); *(tp->error) = true; sem_post(tp->semaphore); return NULL; @@ -121,7 +122,7 @@ static void* frepo_sync_manifest__thread(void* param) char* remote_full = path_join(tp->manifest_url, - tp->manifest->project[p].remote); + project->remote); if (!remote_full) { fprintf(stderr, @@ -133,11 +134,11 @@ static void* frepo_sync_manifest__thread(void* param) } bool update_success = git_update( - tp->manifest->project[p].path, + project->path, remote_full, - tp->manifest->project[p].name, - tp->manifest->project[p].remote_name, - tp->manifest->project[p].revision, tp->mirror); + project->name, + project->remote_name, + project->revision, tp->mirror); unsigned r, d; for (r = 0, d = tp->retry_delay; @@ -147,23 +148,23 @@ static void* frepo_sync_manifest__thread(void* param) fprintf(stderr, "Warning: Failed to %s '%s'" ", waiting %u ms and retrying.\n", (exists ? "update" : "clone"), - tp->manifest->project[p].path, d); + project->path, d); usleep(tp->retry_delay * 1000); update_success = git_update( - tp->manifest->project[p].path, + project->path, remote_full, - tp->manifest->project[p].name, - tp->manifest->project[p].remote_name, - tp->manifest->project[p].revision, tp->mirror); + project->name, + project->remote_name, + project->revision, tp->mirror); } if (!update_success) { fprintf(stderr, "Error: Failed to %s '%s'", (exists ? "update" : "clone"), - tp->manifest->project[p].path); + project->path); if (tp->retries != 0) fprintf(stderr, " after %u retries", tp->retries); fprintf(stderr, ".\n"); @@ -172,36 +173,37 @@ static void* frepo_sync_manifest__thread(void* param) free(remote_full); unsigned j; - for (j = 0; j < tp->manifest->project[p].copyfile_count; j++) + for (j = 0; j < project->copyfile_count; j++) { - char cmd[strlen(tp->manifest->project[p].path) - + strlen(tp->manifest->project[p].copyfile[j].source) - + strlen(tp->manifest->project[p].copyfile[j].dest) + 16]; + copyfile_t* copyfile = &project->copyfile[j]; + char cmd[strlen(project->path) + + strlen(copyfile->source) + + strlen(copyfile->dest) + 16]; sprintf(cmd, "cp %s/%s %s", - tp->manifest->project[p].path, - tp->manifest->project[p].copyfile[j].source, - tp->manifest->project[p].copyfile[j].dest); + project->path, + copyfile->source, + copyfile->dest); if (system(cmd) != EXIT_SUCCESS) { unsigned k; for (k = 0; k < j; k++) - git_remove(tp->manifest->project[k].path); + git_remove(project->path); fprintf(stderr, "Error: Failed to perform copy '%s' to '%s'" " for project '%s'\n", - tp->manifest->project[p].copyfile[j].source, - tp->manifest->project[p].copyfile[j].dest, - tp->manifest->project[p].path); + copyfile->source, + copyfile->dest, + project->path); *(tp->error) = true; } } if (revision_differs && !git_checkout( - tp->manifest->project[p].path, + project->path, revision, false)) { fprintf(stderr, "Error: Failed to revert '%s' to revision '%s'.\n", - tp->manifest->project[p].path, revision); + project->path, revision); *(tp->error) = true; } From 1e88082b81702e9dae50fadef005bf0b72d81212 Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Mon, 15 Jun 2026 17:49:09 +0100 Subject: [PATCH 04/12] Implement linkfile It is specified in https://gerrit.googlesource.com/git-repo/+/refs/tags/v2.64/project.py#471 that if src is a glob then every matching file is symlinked into dest, but if there is no glob then the destination is the target. Since frepo already shells out to run commands instead of doing things in-process it is convenient that the shell and the ln command already implement the specified behaviour. --- src/main.c | 49 +++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 41 insertions(+), 8 deletions(-) diff --git a/src/main.c b/src/main.c index a71c1ae..2dddc21 100644 --- a/src/main.c +++ b/src/main.c @@ -73,6 +73,22 @@ struct manifest_thread_params bool complete; }; +static int do_copyfile( + const char* cmd, + const char* path, + const char* source, + const char* dest) +{ + char buf[strlen(cmd) + + strlen(path) + + strlen(source) + + strlen(dest) + 4]; + sprintf(buf, + "%s %s/%s %s", + cmd, path, source, dest); + return system(buf); +} + static void* frepo_sync_manifest__thread(void* param) { volatile struct manifest_thread_params* tp @@ -176,14 +192,8 @@ static void* frepo_sync_manifest__thread(void* param) for (j = 0; j < project->copyfile_count; j++) { copyfile_t* copyfile = &project->copyfile[j]; - char cmd[strlen(project->path) - + strlen(copyfile->source) - + strlen(copyfile->dest) + 16]; - sprintf(cmd, "cp %s/%s %s", - project->path, - copyfile->source, - copyfile->dest); - if (system(cmd) != EXIT_SUCCESS) + int res = do_copyfile("cp", project->path, copyfile->source, copyfile->dest); + if (res != EXIT_SUCCESS) { unsigned k; for (k = 0; k < j; k++) @@ -198,6 +208,29 @@ static void* frepo_sync_manifest__thread(void* param) } } + for (j = 0; j < project->linkfile_count; j++) + { + copyfile_t* linkfile = &project->linkfile[j]; + int res = do_copyfile( + "ln --symbolic --relative", + project->path, + linkfile->source, + linkfile->dest); + if (res != EXIT_SUCCESS) + { + unsigned k; + for (k = 0; k < j; k++) + git_remove(project->path); + fprintf(stderr, + "Error: Failed to perform link '%s' to '%s'" + " for project '%s'\n", + linkfile->source, + linkfile->dest, + project->path); + *(tp->error) = true; + } + } + if (revision_differs && !git_checkout( project->path, revision, false)) From a890beb513dc5d2ce12cfa6845714704b0f0f37b Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Fri, 19 Jun 2026 17:54:03 +0100 Subject: [PATCH 05/12] Make filter logic compatible with android git-repo The previous logic had a bug where projects with non-empty lists were considered to not be part of the default group, since the check was just whether the project had no groups or the group contained default, when the logic is that anything that doesn't have notdefault is implicitly part of the default group. --- src/main.c | 16 ++++++++++ src/manifest.c | 79 ++++++++++++++++++++++++++++++-------------------- 2 files changed, 63 insertions(+), 32 deletions(-) diff --git a/src/main.c b/src/main.c index 2dddc21..8c31d12 100644 --- a/src/main.c +++ b/src/main.c @@ -965,6 +965,22 @@ int main(int argc, char* argv[]) } } + // If we didn't receive groups on the command-line or settings + // default it to "default". + if (settings->group == NULL) + { + if(!group_list_add( + "default", + strlen("default"), + false /*not excluded*/, + &settings->group, + &settings->group_count)) + { + fprintf(stderr, "Failed to add \"default\" filter group.\n"); + return EXIT_FAILURE; + } + } + if (command == frepo_command_init) { if (!settings_manifest_url_set( diff --git a/src/manifest.c b/src/manifest.c index 1dd14f0..e5cf903 100644 --- a/src/manifest.c +++ b/src/manifest.c @@ -661,46 +661,61 @@ manifest_t* manifest_group_filter( if (!manifest) return NULL; - bool include_default = true; - bool include_all = false; - - unsigned i; - if (group_list_match( - "default", strlen("default"), - filter, filter_count, &i)) - include_default = !filter[i].exclude; - if (group_list_match( - "all", strlen("all"), - filter, filter_count, &i)) - include_all = !filter[i].exclude; - bool mask[manifest->project_count]; + unsigned i; unsigned project_count = 0; for (i = 0; i < manifest->project_count; i++) { - mask[i] = include_all; - if ((manifest->project[i].group_count == 0) - || (group_list_match( - "default", strlen("default"), - manifest->project[i].group, - manifest->project[i].group_count, NULL))) - { - mask[i] |= include_default; - } - else if (filter) + project_t* project = &manifest->project[i]; + mask[i] = false; + // Process the filter in reverse since the result only depends + // on the last match + for (group_t* rule = &filter[filter_count - 1]; + rule >= filter; + rule--) { - unsigned j; - for (j = 0; j < filter_count; j++) + // TODO: Should pre-parsing have an enum for rule->kind + // as ALL, DEFAULT or NAMED to reduce redundant parsing? + if (strcmp(rule->name, "all") == 0) { - unsigned m; - if (group_list_match( - filter[j].name, filter[j].size, - manifest->project[i].group, - manifest->project[i].group_count, - &m)) - mask[i] = !filter[j].exclude; + mask[i] = !rule->exclude; + break; } + + bool matched = false; + if (strcmp(rule->name, "default") == 0) + { + // Default implicitly exists if + // notdefault does not + if (!group_list_match( + "notdefault", + strlen("notdefault"), + project->group, + project->group_count, + NULL)) + { + mask[i] = !rule->exclude; + matched = true; + } + // Perverse input may include both + // default and notdefault so we need to + // fall through to check if default + // also exists. + } + + if (group_list_match( + rule->name, + rule->size, + project->group, + project->group_count, + NULL)) + { + mask[i] = !rule->exclude; + matched = true; + } + + if (matched) break; } if (mask[i]) From 01ba7a688c940a12c8513a1c1a3d2750944d6da0 Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Thu, 25 Jun 2026 17:47:04 +0100 Subject: [PATCH 06/12] Make the Makefile more resilient to vim .swp files --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index f92c6e6..05c552d 100644 --- a/Makefile +++ b/Makefile @@ -8,7 +8,7 @@ CFLAGS_DEBUG = -O0 -g $(CFLAGS_COMMON) LDFLAGS_DEBUG = -lm -lexpat LDFLAGS_RELEASE = -s $(LDFLAGS_DEBUG) -SRC = $(shell find src -type f) +SRC = $(shell find src -type f -name "*.c") OBJ_RELEASE = $(patsubst src/%.c, .build/%.o, $(SRC)) DEP_RELEASE = $(patsubst src/%.c, .build/%.d, $(SRC)) OBJ_DEBUG = $(patsubst src/%.c, .build/debug/%.o, $(SRC)) From 1772250130066e9a5ba9e0fee9dc432ee413fa49 Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Thu, 25 Jun 2026 17:54:24 +0100 Subject: [PATCH 07/12] Add support for filtering by platform groups --- include/group.h | 4 +++ src/group.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++++ src/main.c | 42 ++++++++++++++++++++++++++-- 3 files changed, 116 insertions(+), 3 deletions(-) diff --git a/include/group.h b/include/group.h index 8303aa6..30a1dd4 100644 --- a/include/group.h +++ b/include/group.h @@ -47,4 +47,8 @@ extern bool group_list_parse( const char* groups, bool filter, group_t** list, unsigned* list_count); +extern const group_t* group_list_parse_platform( + const char* platform, + unsigned* platform_groups_count); + #endif diff --git a/src/group.c b/src/group.c index a705df4..0fecfca 100644 --- a/src/group.c +++ b/src/group.c @@ -21,6 +21,7 @@ #include #include +#include bool group_list_match( @@ -194,3 +195,75 @@ bool group_list_parse( *list_count = nlist_count; return true; } + + +// Sorted in order of least common to most since groups can be processed +// in reverse order and use the last match, so it's more likely to match faster +// if the most common is last. +static const group_t platform_groups[] = +{ + { + "platform-darwin", + 15, + false, + }, + { + "platform-windows", + 16, + false, + }, + { + "platform-linux", + 14, + false, + } +}; +#define PLATFORM_GROUPS_COUNT (sizeof(platform_groups)/sizeof(platform_groups[0])) + + +const group_t* group__list_find_by_name( + const char* platform, + unsigned* platform_groups_count) +{ + const group_t* match; + for (match = &platform_groups[PLATFORM_GROUPS_COUNT - 1]; + match >= platform_groups; + match--) + { + if (strncasecmp( + platform, + &match->name[strlen("platform-")], + match->size - strlen("platform-")) == 0) + { + *platform_groups_count = 1; + return match; + } + } + return NULL; +} + + +const group_t* group_list_parse_platform( + const char* platform, + unsigned* platform_groups_count) +{ + if (strcmp(platform, "all") == 0) + { + *platform_groups_count = PLATFORM_GROUPS_COUNT; + return platform_groups; + } + + if (strcmp(platform, "auto") == 0) + { + struct utsname u; + if (uname(&u) != 0) + // This should never happen in practise + // because &u is always a valid pointer, + return NULL; + return group__list_find_by_name( + u.sysname, + platform_groups_count); + } + + return group__list_find_by_name(platform, platform_groups_count); +} diff --git a/src/main.c b/src/main.c index 8c31d12..25ea79a 100644 --- a/src/main.c +++ b/src/main.c @@ -774,6 +774,7 @@ int main(int argc, char* argv[]) bool force = false; bool print = false; long int threads = 0; + const char* platform = "auto"; const char* settings_path = ".frepo/config.ini"; settings_t* settings = settings_read(settings_path); @@ -895,15 +896,21 @@ int main(int argc, char* argv[]) a = (argc - 1); break; case 'p': - if (command != frepo_command_forall) + if (command == frepo_command_forall) + { + print = true; + break; + } + + if ((a + 1) >= argc) { fprintf(stderr, - "Error: -p flag invalid for command.\n"); + "Error: No platform supplied with platform flag.\n"); print_usage(argv[0]); return EXIT_FAILURE; } - print = true; + platform = argv[++a]; break; case 'f': if (command != frepo_command_sync) @@ -981,6 +988,35 @@ int main(int argc, char* argv[]) } } + const group_t* platform_groups; + unsigned platform_groups_count; + platform_groups = group_list_parse_platform( + platform, &platform_groups_count); + if (!platform_groups) + { + fprintf(stderr, + "Error: unrecognized platform \"%s\".\n", + platform); + print_usage(argv[0]); + return EXIT_FAILURE; + } + for (; platform_groups_count >= 1; platform_groups_count--, platform_groups++) + { + if (!group_list_add( + platform_groups->name, + platform_groups->size, + platform_groups->exclude, + &settings->group, + &settings->group_count)) + { + fprintf(stderr, + "Failed to add \"%*s\" filter group.\n", + (int)platform_groups->size, + platform_groups->name); + return EXIT_FAILURE; + } + } + if (command == frepo_command_init) { if (!settings_manifest_url_set( From 27dabcd8a8dbe47ffe23f04a34dc65a89c00600c Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Fri, 26 Jun 2026 16:06:13 +0100 Subject: [PATCH 08/12] Ignore repo-hooks elements --- src/manifest.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/manifest.c b/src/manifest.c index e5cf903..9481e83 100644 --- a/src/manifest.c +++ b/src/manifest.c @@ -71,6 +71,11 @@ manifest_t* manifest_parse(xml_tag_t* document) remote_count++; else if (strcmp(mdoc->tag[i]->name, "project") == 0) project_count++; + else if (strcmp(mdoc->tag[i]->name, "repo-hooks") == 0) + // Hooks are python functions loaded from file and lose + // the benefits of a C implementation. + fprintf(stderr, + "Warning: repo python hooks are ignored.\n"); else if (strcmp(mdoc->tag[i]->name, "default") != 0) { fprintf(stderr, From 2218571444a8b764331e97f806bbd430aa27441e Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Fri, 26 Jun 2026 16:43:50 +0100 Subject: [PATCH 09/12] Ignore bugurl elements introduced in android 12 We don't need to do anything with them. --- src/manifest.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/manifest.c b/src/manifest.c index 9481e83..347fef4 100644 --- a/src/manifest.c +++ b/src/manifest.c @@ -76,6 +76,9 @@ manifest_t* manifest_parse(xml_tag_t* document) // the benefits of a C implementation. fprintf(stderr, "Warning: repo python hooks are ignored.\n"); + else if (strcmp(mdoc->tag[i]->name, "bugurl") == 0) + fprintf(stderr, + "Warning: bugurl is ignored.\n"); else if (strcmp(mdoc->tag[i]->name, "default") != 0) { fprintf(stderr, From 995f2ec23a82f64e5512b5900702d35f89b1d431 Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Fri, 26 Jun 2026 16:52:44 +0100 Subject: [PATCH 10/12] Ignore superproject element Its use for sync is optional and can be opted out in android git-repo with the --no-use-superproject option so we behave as if that's always provided. --- src/manifest.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/manifest.c b/src/manifest.c index 347fef4..620b337 100644 --- a/src/manifest.c +++ b/src/manifest.c @@ -79,6 +79,12 @@ manifest_t* manifest_parse(xml_tag_t* document) else if (strcmp(mdoc->tag[i]->name, "bugurl") == 0) fprintf(stderr, "Warning: bugurl is ignored.\n"); + else if (strcmp(mdoc->tag[i]->name, "superproject") == 0) + // Use of superproject to sync can be opted out with + // the --no-use-superproject option of android git-repo + // so we behave as if it was always provided. + fprintf(stderr, + "Warning: superprojects are not supported.\n"); else if (strcmp(mdoc->tag[i]->name, "default") != 0) { fprintf(stderr, From 86cdd16e4b66533b0060ab611b4908b819f81fdc Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Fri, 26 Jun 2026 16:55:11 +0100 Subject: [PATCH 11/12] Ignore contactinfo elements We don't need to do anything with them. --- src/manifest.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/manifest.c b/src/manifest.c index 620b337..10fccde 100644 --- a/src/manifest.c +++ b/src/manifest.c @@ -79,6 +79,9 @@ manifest_t* manifest_parse(xml_tag_t* document) else if (strcmp(mdoc->tag[i]->name, "bugurl") == 0) fprintf(stderr, "Warning: bugurl is ignored.\n"); + else if (strcmp(mdoc->tag[i]->name, "contactinfo") == 0) + fprintf(stderr, + "Warning: contactinfo is ignored.\n"); else if (strcmp(mdoc->tag[i]->name, "superproject") == 0) // Use of superproject to sync can be opted out with // the --no-use-superproject option of android git-repo From 20ac6baf0173d980a9d35f2e53bff7a237586c6b Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Fri, 26 Jun 2026 17:10:35 +0100 Subject: [PATCH 12/12] Warn if a project uses clone-depth clone-depth=1 is an optimisation that should be implemented but isn't and it's better to have a warning than it be silently ignored. --- src/manifest.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/manifest.c b/src/manifest.c index 10fccde..ed380d2 100644 --- a/src/manifest.c +++ b/src/manifest.c @@ -184,6 +184,16 @@ manifest_t* manifest_parse(xml_tag_t* document) project->remote = xml_tag_field(mdoc->tag[i], "remote"); + + if (xml_tag_field(mdoc->tag[i], "clone-depth")) + { + static bool warned = false; + if (!warned) + { + fprintf(stderr, "Warning: clone-depth is ignored.\n"); + warned = true; + } + } if (project->remote) { unsigned r;