Re: [PATCH 4/4] replace: add a --raw mode for --edit
On Wed, Jun 25, 2014 at 03:33:42PM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: One of the purposes of git replace --edit is to help a user repair objects which are malformed or corrupted. Usually we pretty-print trees with ls-tree, which is much easier to work with than the raw binary data. However, some forms of corruption break the tree-walker, in which case our pretty-printing fails, rendering --edit useless for the user. This patch introduces a --raw option, which lets you edit the binary data in these instances. Signed-off-by: Jeff King p...@peff.net --- Hm, this feels almost like inviting accidents to make it easy and limiting the attempt to only one shot at the transformation step. Will queue, but we can do the same with cat-file $type temp, do some transformation on temp, doubly make sure what is in temp is sensible and then finally hash-object -w -t $type temp. And it makes me feel uneasy that the new feature seems to make it harder to do the doubly make sure part. I do not think it is any worse than --edit is by itself. True, editing the binary contents is hard, but we do check that the format is sane when we read it back in (using the same checks that hash-object does). I think it would be nice to take that a step further and actually let hash-object (and replace --edit) do the more rigorous fsck checks on created objects. I do still think even with those automated sanity checks that it makes sense to double-check the replacement manually. But I think that is one of the features of replace objects: you are not doing anything permanent, and can view the object in place. So not only can you examine the object by git show $new_sha1, you can see it in place as git log -p, etc. And easily back it out with git replace -d (or fix it up again with git replace --edit) if need be. -Peff -- 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 4/4] replace: add a --raw mode for --edit
On Tue, Jun 24, 2014 at 09:40:09PM -0400, Eric Sunshine wrote: On Tue, Jun 24, 2014 at 5:46 AM, Jeff King p...@peff.net wrote: One of the purposes of git replace --edit is to help a user repair objects which are malformed or corrupted. Usually we pretty-print trees with ls-tree, which is much easier to work with than the raw binary data. However, some forms of corruption break the tree-walker, in which case our pretty-printing fails, rendering --edit useless for the user. This patch introduces a --raw option, which lets you edit the binary data in these instances. Is there a possibility that any of the other git-replace modes will grow a need for raw? If not, would it make sense to make this specific to edit as --edit=raw? I doubt that any other modes will want it, as it is about the pretty-printing step which is pretty specific to --edit. However, making it --edit=raw also precludes adding other modes to --edit. I do not have any in mind, but I do not see it as impossible. Preclude is maybe a strong word. You could have --edit=raw,flag1,flag2, but then we are essentially reinventing an option parser inside --edit's value. Not to mention that you cannot do --no-raw, even without other flags being added. -Peff -- 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 4/4] replace: add a --raw mode for --edit
Jeff King p...@peff.net writes:
One of the purposes of git replace --edit is to help a
user repair objects which are malformed or corrupted.
Usually we pretty-print trees with ls-tree, which is much
easier to work with than the raw binary data. However, some
forms of corruption break the tree-walker, in which case our
pretty-printing fails, rendering --edit useless for the
user.
This patch introduces a --raw option, which lets you edit
the binary data in these instances.
Signed-off-by: Jeff King p...@peff.net
---
Hm, this feels almost like inviting accidents to make it easy
and limiting the attempt to only one shot at the transformation
step.
Will queue, but we can do the same with cat-file $type temp, do
some transformation on temp, doubly make sure what is in temp is
sensible and then finally hash-object -w -t $type temp. And it
makes me feel uneasy that the new feature seems to make it harder to
do the doubly make sure part.
I dunno.
Documentation/git-replace.txt | 8
builtin/replace.c | 31 +--
2 files changed, 29 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index 61461b9..089dcac 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -73,6 +73,14 @@ OPTIONS
newly created object. See linkgit:git-var[1] for details about
how the editor will be chosen.
+--raw::
+ When editing, provide the raw object contents rather than
+ pretty-printed ones. Currently this only affects trees, which
+ will be shown in their binary form. This is harder to work with,
+ but can help when repairing a tree that is so corrupted it
+ cannot be pretty-printed. Note that you may need to configure
+ your editor to cleanly read and write binary data.
+
-l pattern::
--list pattern::
List replace refs for objects that match the given pattern (or
diff --git a/builtin/replace.c b/builtin/replace.c
index 2584170..d1ea2c2 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -188,10 +188,12 @@ static int replace_object(const char *object_ref, const
char *replace_ref, int f
}
/*
- * Write the contents of the object named by sha1 to the file filename,
- * pretty-printed for human editing based on its type.
+ * Write the contents of the object named by sha1 to the file filename.
+ * If raw is true, then the object's raw contents are printed according to
+ * type. Otherwise, we pretty-print the contents for human editing.
*/
-static void export_object(const unsigned char *sha1, const char *filename)
+static void export_object(const unsigned char *sha1, enum object_type type,
+ int raw, const char *filename)
{
struct child_process cmd = { NULL };
int fd;
@@ -202,7 +204,10 @@ static void export_object(const unsigned char *sha1,
const char *filename)
argv_array_push(cmd.args, --no-replace-objects);
argv_array_push(cmd.args, cat-file);
- argv_array_push(cmd.args, -p);
+ if (raw)
+ argv_array_push(cmd.args, typename(type));
+ else
+ argv_array_push(cmd.args, -p);
argv_array_push(cmd.args, sha1_to_hex(sha1));
cmd.git_cmd = 1;
cmd.out = fd;
@@ -217,7 +222,7 @@ static void export_object(const unsigned char *sha1,
const char *filename)
* The sha1 of the written object is returned via sha1.
*/
static void import_object(unsigned char *sha1, enum object_type type,
- const char *filename)
+ int raw, const char *filename)
{
int fd;
@@ -225,7 +230,7 @@ static void import_object(unsigned char *sha1, enum
object_type type,
if (fd 0)
die_errno(unable to open %s for reading, filename);
- if (type == OBJ_TREE) {
+ if (!raw type == OBJ_TREE) {
const char *argv[] = { mktree, NULL };
struct child_process cmd = { argv };
struct strbuf result = STRBUF_INIT;
@@ -265,7 +270,7 @@ static void import_object(unsigned char *sha1, enum
object_type type,
*/
}
-static int edit_and_replace(const char *object_ref, int force)
+static int edit_and_replace(const char *object_ref, int force, int raw)
{
char *tmpfile = git_pathdup(REPLACE_EDITOBJ);
enum object_type type;
@@ -281,10 +286,10 @@ static int edit_and_replace(const char *object_ref, int
force)
check_ref_valid(old, prev, ref, sizeof(ref), force);
- export_object(old, tmpfile);
+ export_object(old, type, raw, tmpfile);
if (launch_editor(tmpfile, NULL, NULL) 0)
die(editing object file failed);
- import_object(new, type, tmpfile);
+ import_object(new, type, raw, tmpfile);
free(tmpfile);
@@ -297,6 +302,7 @@ static int edit_and_replace(const char *object_ref, int
force)
int cmd_replace(int
Re: [PATCH 4/4] replace: add a --raw mode for --edit
On Tue, Jun 24, 2014 at 5:46 AM, Jeff King p...@peff.net wrote:
One of the purposes of git replace --edit is to help a
user repair objects which are malformed or corrupted.
Usually we pretty-print trees with ls-tree, which is much
easier to work with than the raw binary data. However, some
forms of corruption break the tree-walker, in which case our
pretty-printing fails, rendering --edit useless for the
user.
This patch introduces a --raw option, which lets you edit
the binary data in these instances.
Is there a possibility that any of the other git-replace modes will
grow a need for raw? If not, would it make sense to make this
specific to edit as --edit=raw?
Signed-off-by: Jeff King p...@peff.net
---
Documentation/git-replace.txt | 8
builtin/replace.c | 31 +--
2 files changed, 29 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index 61461b9..089dcac 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -73,6 +73,14 @@ OPTIONS
newly created object. See linkgit:git-var[1] for details about
how the editor will be chosen.
+--raw::
+ When editing, provide the raw object contents rather than
+ pretty-printed ones. Currently this only affects trees, which
+ will be shown in their binary form. This is harder to work with,
+ but can help when repairing a tree that is so corrupted it
+ cannot be pretty-printed. Note that you may need to configure
+ your editor to cleanly read and write binary data.
+
-l pattern::
--list pattern::
List replace refs for objects that match the given pattern (or
diff --git a/builtin/replace.c b/builtin/replace.c
index 2584170..d1ea2c2 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -188,10 +188,12 @@ static int replace_object(const char *object_ref, const
char *replace_ref, int f
}
/*
- * Write the contents of the object named by sha1 to the file filename,
- * pretty-printed for human editing based on its type.
+ * Write the contents of the object named by sha1 to the file filename.
+ * If raw is true, then the object's raw contents are printed according to
+ * type. Otherwise, we pretty-print the contents for human editing.
*/
-static void export_object(const unsigned char *sha1, const char *filename)
+static void export_object(const unsigned char *sha1, enum object_type type,
+ int raw, const char *filename)
{
struct child_process cmd = { NULL };
int fd;
@@ -202,7 +204,10 @@ static void export_object(const unsigned char *sha1,
const char *filename)
argv_array_push(cmd.args, --no-replace-objects);
argv_array_push(cmd.args, cat-file);
- argv_array_push(cmd.args, -p);
+ if (raw)
+ argv_array_push(cmd.args, typename(type));
+ else
+ argv_array_push(cmd.args, -p);
argv_array_push(cmd.args, sha1_to_hex(sha1));
cmd.git_cmd = 1;
cmd.out = fd;
@@ -217,7 +222,7 @@ static void export_object(const unsigned char *sha1,
const char *filename)
* The sha1 of the written object is returned via sha1.
*/
static void import_object(unsigned char *sha1, enum object_type type,
- const char *filename)
+ int raw, const char *filename)
{
int fd;
@@ -225,7 +230,7 @@ static void import_object(unsigned char *sha1, enum
object_type type,
if (fd 0)
die_errno(unable to open %s for reading, filename);
- if (type == OBJ_TREE) {
+ if (!raw type == OBJ_TREE) {
const char *argv[] = { mktree, NULL };
struct child_process cmd = { argv };
struct strbuf result = STRBUF_INIT;
@@ -265,7 +270,7 @@ static void import_object(unsigned char *sha1, enum
object_type type,
*/
}
-static int edit_and_replace(const char *object_ref, int force)
+static int edit_and_replace(const char *object_ref, int force, int raw)
{
char *tmpfile = git_pathdup(REPLACE_EDITOBJ);
enum object_type type;
@@ -281,10 +286,10 @@ static int edit_and_replace(const char *object_ref, int
force)
check_ref_valid(old, prev, ref, sizeof(ref), force);
- export_object(old, tmpfile);
+ export_object(old, type, raw, tmpfile);
if (launch_editor(tmpfile, NULL, NULL) 0)
die(editing object file failed);
- import_object(new, type, tmpfile);
+ import_object(new, type, raw, tmpfile);
free(tmpfile);
@@ -297,6 +302,7 @@ static int edit_and_replace(const char *object_ref, int
force)
int cmd_replace(int argc, const char **argv, const char *prefix)
{
int force = 0;
+ int raw = 0;
const char *format = NULL;
enum {
MODE_UNSPECIFIED =
