Re: [PATCH v5 1/7] replace: forbid replacing an object with one of a different type

[Junio C Hamano]

Christian Couder chrisc...@tuxfamily.org writes:

 + obj_type = sha1_object_info(object, NULL);
 + repl_type = sha1_object_info(repl, NULL);

That we can do this is somewhat curious.  If an object is replaced
with a different one, would it mean that this code snippet is
totally bogus?

type1 = sha1_object_info(sha1, size1);
area = xmalloc(size1);
orig = read_sha1_file(sha1, type0, size0);
memcpy(area, orig, size1);
free(orig);

 + if (!force  obj_type != repl_type)
 + die(Objects must be of the same type.\n
 + '%s' points to a replaced object of type '%s'\n
 + while '%s' points to a replacement object of type '%s'.,
 + object_ref, typename(obj_type),
 + replace_ref, typename(repl_type));
 +
   if (read_ref(ref, prev))
   hashclr(prev);
   else if (!force)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 1/7] replace: forbid replacing an object with one of a different type

[Junio C Hamano]

Junio C Hamano gits...@pobox.com writes:

 Christian Couder chrisc...@tuxfamily.org writes:

 +obj_type = sha1_object_info(object, NULL);
 +repl_type = sha1_object_info(repl, NULL);

 That we can do this is somewhat curious

Note that this was a comment on the sha1_object_info() API, which,
unlike read_sha1_file() API, does not seem to interact with the
replace mechanism.  I _think_ that needs to be rethought by
checking each call site, but for the purpose of this series, I think
this is the best we can do in this patch.

Will queue the whole series.

Thanks.


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 1/7] replace: forbid replacing an object with one of a different type

[Christian Couder]

Users replacing an object with one of a different type were not
prevented to do so, even if it was obvious, and stated in the doc,
that bad things would result from doing that.

To avoid mistakes, it is better to just forbid that though.

If -f option, which means '--force', is used, we can allow an object
to be replaced with one of a different type, as the user should know
what (s)he is doing.

If one object is replaced with one of a different type, the only way
to keep the history valid is to also replace all the other objects
that point to the replaced object. That's because:

* Annotated tags contain the type of the tagged object.

* The tree/parent lines in commits must be a tree and commits, resp.

* The object types referred to by trees are specified in the 'mode'
  field:
100644 and 100755blob
16   commit
04   tree
  (these are the only valid modes)

* Blobs don't point at anything.

The doc will be updated in a later patch.

Acked-by: Philip Oakley philipoak...@iee.org
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 builtin/replace.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/builtin/replace.c b/builtin/replace.c
index 59d3115..95736d9 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -85,6 +85,7 @@ static int replace_object(const char *object_ref, const char 
*replace_ref,
  int force)
 {
unsigned char object[20], prev[20], repl[20];
+   enum object_type obj_type, repl_type;
char ref[PATH_MAX];
struct ref_lock *lock;
 
@@ -100,6 +101,15 @@ static int replace_object(const char *object_ref, const 
char *replace_ref,
if (check_refname_format(ref, 0))
die('%s' is not a valid ref name., ref);
 
+   obj_type = sha1_object_info(object, NULL);
+   repl_type = sha1_object_info(repl, NULL);
+   if (!force  obj_type != repl_type)
+   die(Objects must be of the same type.\n
+   '%s' points to a replaced object of type '%s'\n
+   while '%s' points to a replacement object of type '%s'.,
+   object_ref, typename(obj_type),
+   replace_ref, typename(repl_type));
+
if (read_ref(ref, prev))
hashclr(prev);
else if (!force)
-- 
1.8.4.rc1.28.ge2684af


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html