Hi,
Is there anyone that's familiar with Fossil's import and export system
that would be able/willing to review this? Other than code reviews are
there any steps I should take to increase the likelihood of the patch
being accepted?
Thanks!
-Nick
Nickolas Lloyd writes:
> Hi,
>
> Following is a patch against fossil commit [4d1f2d302b] that fixes an
> issue that can appear when performing multiple imports/exports with a
> git repository.
>
> The problem arises when fossil tries to export a blob to git with a mark
> which has been previously used by git when importing a commit to
> fossil. This happens because fossil creates marks based solely on the
> rid of the object (2*rid for blobs, 2*rid+1 for commits), which can
> clash with git-created marks.
>
> E.g. if we import the following two objects from git:
> blob :19 as rid=10
> commit :20 as rid=11
> and we then later export a commit referring to rid=10, we get something
> like the following:
> commit refs/heads/trunk
> mark :25
> ...
> M 100644 :20 file.txt
> ...
> because fossil thinks that a blob with rid=10 should be marked as `:20'.
> This produces an error from git that mark `:20' is not a blob, but
> actually a commit. This is fairly easy to work around, we just have to
> keep track of all marks that have been declared previously and make sure
> that any new ones that we create are unique.
>
> Please let me know if you have any questions/comments. I'd be happy to
> hear your feedback.
>
> Thanks,
> Nick
>
> ---
>
> Index: src/export.c
> ==
> --- src/export.c
> +++ src/export.c
> @@ -132,23 +132,28 @@
>
> /*
> ** create_mark()
> ** Create a new (mark,rid,uuid) entry for the given rid in the 'xmark'
> table,
> ** and return that information as a struct mark_t in *mark.
> +** *unused_mark is a value representing a mark that is free for use--that
> is,
> +** it does not appear in the marks file, and has not been used during this
>
> +** export run. Specifically, it is the supremum of the set of used marks
>
> +** plus one.
>
> ** This function returns -1 in the case where 'rid' does not exist,
> otherwise
> ** it returns 0.
> ** mark->name is dynamically allocated and is owned by the caller upon
> return.
> */
> -int create_mark(int rid, struct mark_t *mark){
> +int create_mark(int rid, struct mark_t *mark, unsigned int *unused_mark){
>char sid[13];
>char *zUuid = rid_to_uuid(rid);
>if(!zUuid){
> fossil_trace("Undefined rid=%d\n", rid);
> return -1;
>}
>mark->rid = rid;
> - sqlite3_snprintf(sizeof(sid), sid, ":%d", COMMITMARK(rid));
> + sqlite3_snprintf(sizeof(sid), sid, ":%d", *unused_mark);
> + *unused_mark += 1;
>mark->name = fossil_strdup(sid);
>sqlite3_snprintf(sizeof(mark->uuid), mark->uuid, "%s", zUuid);
>free(zUuid);
>insert_commit_xref(mark->rid, mark->name, mark->uuid);
>return 0;
> @@ -156,19 +161,22 @@
>
> /*
> ** mark_name_from_rid()
> ** Find the mark associated with the given rid. Mark names always start
> ** with ':', and are pulled from the 'xmark' temporary table.
> -** This function returns NULL if the rid does not exist in the 'xmark'
> table.
> -** Otherwise, it returns the name of the mark, which is dynamically
> allocated
> -** and is owned by the caller of this function.
> +** If the given rid doesn't have a mark associated with it yet, one is
> +** created with a value of *unused_mark.
> +** *unused_mark functions exactly as in create_mark().
> +** This function returns NULL if the rid does not have an associated UUID,
> +** (i.e. is not valid). Otherwise, it returns the name of the mark, which
> is
> +** dynamically allocated and is owned by the caller of this function.
> */
> -char * mark_name_from_rid(int rid){
> +char * mark_name_from_rid(int rid, unsigned int *unused_mark){
>char *zMark = db_text(0, "SELECT tname FROM xmark WHERE trid=%d", rid);
>if(zMark==NULL){
> struct mark_t mark;
> -if(create_mark(rid, &mark)==0){
> +if(create_mark(rid, &mark, unused_mark)==0){
>zMark = mark.name;
> }else{
>return NULL;
> }
>}
> @@ -185,16 +193,18 @@
> ** database. Otherwise, 0 is returned.
> ** mark->name is dynamically allocated, and owned by the caller.
> */
> int parse_mark(char *line, struct mark_t *mark){
>char *cur_tok;
> + char type_;
>cur_tok = strtok(line, " \t");
>if(!cur_tok||strlen(cur_tok)<2){
> return -1;
>}
>mark->rid = atoi(&cur_tok[1]);
> - if(cur_tok[0]!='c'){
> + type_ = cur_tok[0];
> + if(type_!='c'&&type_!='b'){
> /* This is probably a blob mark */
> mark->name = NULL;
> return 0;
>}
>
> @@ -202,11 +212,18 @@
>if(!cur_tok){
> /* This mark was generated by an older version of Fossil and doesn't
> ** incl
