From: Adam Roben <hidden> Date: 2016-06-15 22:43:44
I separated the logic of parsing the arguments from the logic of fetching and
outputting the data. cat_one_file now does the latter.
Signed-off-by: Adam Roben <redacted>
---
builtin-cat-file.c | 38 ++++++++++++++++++++++----------------
1 files changed, 22 insertions(+), 16 deletions(-)
@@ -76,31 +76,16 @@ static void pprint_tag(const unsigned char *sha1, const char *buf, unsigned longwrite_or_die(1,cp,endp-cp);}-intcmd_cat_file(intargc,constchar**argv,constchar*prefix)+staticintcat_one_file(intopt,constchar*exp_type,constchar*obj_name){unsignedcharsha1[20];enumobject_typetype;void*buf;unsignedlongsize;-intopt;-constchar*exp_type,*obj_name;--git_config(git_default_config);-if(argc!=3)-usage("git-cat-file [-t|-s|-e|-p|<type>] <sha1>");-exp_type=argv[1];-obj_name=argv[2];if(get_sha1(obj_name,sha1))die("Not a valid object name %s",obj_name);-opt=0;-if(exp_type[0]=='-'){-opt=exp_type[1];-if(!opt||exp_type[2])-opt=-1;/* Not a single character option */-}-buf=NULL;switch(opt){case't':
@@ -157,3 +142,24 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)write_or_die(1,buf,size);return0;}++intcmd_cat_file(intargc,constchar**argv,constchar*prefix)+{+intopt;+constchar*exp_type,*obj_name;++git_config(git_default_config);+if(argc!=3)+usage("git-cat-file [-t|-s|-e|-p|<type>] <sha1>");+exp_type=argv[1];+obj_name=argv[2];++opt=0;+if(exp_type[0]=='-'){+opt=exp_type[1];+if(!opt||exp_type[2])+opt=-1;/* Not a single character option */+}++returncat_one_file(opt,exp_type,obj_name);+}
From: Adam Roben <hidden> Date: 2016-06-15 22:43:44
This will make it easier to add newer options later.
Signed-off-by: Adam Roben <redacted>
---
builtin-cat-file.c | 42 ++++++++++++++++++++++++++++++------------
1 files changed, 30 insertions(+), 12 deletions(-)
@@ -143,23 +143,41 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)return0;}+staticconstcharcat_file_usage[]="git-cat-file [-t|-s|-e|-p|<type>] <sha1>";+intcmd_cat_file(intargc,constchar**argv,constchar*prefix){-intopt;-constchar*exp_type,*obj_name;+inti,opt=0;+constchar*exp_type=0,*obj_name=0;git_config(git_default_config);-if(argc!=3)-usage("git-cat-file [-t|-s|-e|-p|<type>] <sha1>");-exp_type=argv[1];-obj_name=argv[2];--opt=0;-if(exp_type[0]=='-'){-opt=exp_type[1];-if(!opt||exp_type[2])-opt=-1;/* Not a single character option */++for(i=1;i<argc;++i){+constchar*arg=argv[i];++if(!strcmp(arg,"-t")||!strcmp(arg,"-s")||!strcmp(arg,"-e")||!strcmp(arg,"-p")){+exp_type=arg;+opt=exp_type[1];+continue;+}++if(arg[0]=='-')+usage(cat_file_usage);++if(!exp_type){+exp_type=arg;+continue;+}++if(obj_name)+usage(cat_file_usage);++obj_name=arg;+break;}+if(!exp_type||!obj_name)+usage(cat_file_usage);+returncat_one_file(opt,exp_type,obj_name);}
From: Adam Roben <hidden> Date: 2016-06-15 22:43:44
This lets you specify object names on stdin instead of on the command line.
When printing object contents or pretty-printing, objects will be printed
preceded by their size:
<size>LF
<content>LF
Signed-off-by: Adam Roben <redacted>
---
Brian Downing wrote:
I think a far more reasonable output format for multiple objects would
be something like:
<count> LF
<raw data> LF
Where <count> is the number of bytes in the <raw data> as an ASCII
decimal integer.
@@ -8,7 +8,7 @@ git-cat-file - Provide content or type/size information for repository objects SYNOPSIS ---------'git-cat-file' [-t | -s | -e | -p | <type>] <object>+'git-cat-file' [-t | -s | -e | -p | <type>] [--stdin | <object>] DESCRIPTION -----------
@@ -23,6 +23,10 @@ OPTIONS For a more complete list of ways to spell object names, see "SPECIFYING REVISIONS" section in gitlink:git-rev-parse[1].+--stdin::+ Read object names from stdin instead of specifying one on the+ command line.+ -t:: Instead of the content, show the object type identified by <object>.
From: Adam Roben <hidden> Date: 2016-06-15 22:43:44
command_bidi_pipe hands back the stdin and stdout file handles from the
executed command. command_close_bidi_pipe closes these handles and terminates
the process.
Signed-off-by: Adam Roben <redacted>
---
perl/Git.pm | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 56 insertions(+), 0 deletions(-)
@@ -51,6 +51,7 @@ require Exporter;# Methods which can be called as standalone functions as well:@EXPORT_OK=qw(commandcommand_onelinecommand_noisycommand_output_pipecommand_input_pipecommand_close_pipe+command_bidi_pipecommand_close_bidi_pipeversionexec_pathhash_objectgit_cmd_try);
From: Adam Roben <hidden> Date: 2016-06-15 22:43:44
We were spending a lot of time forking/execing git-cat-file and
git-hash-object. We now maintain a global Git repository object in order to use
Git.pm's more efficient hash_and_insert_object and cat_blob methods.
Signed-off-by: Adam Roben <redacted>
---
Eric Wong wrote:
Related to the above. It's better to sysread()/syswrite() or
read()/print() in a loop with a predefined buffer size rather than to
use a readline() since you could be dealing with files with very long
lines or binaries with no newline characters in them at all.
@@ -332,6 +333,7 @@ sub cmd_init {"as a command-line argument\n";init_subdir(@_);do_git_init_db();+$_repository=Git->repository(Repository=>$ENV{GIT_DIR});Git::SVN->init($url);}
@@ -2541,6 +2543,7 @@ use vars qw/@ISA/;usestrict;usewarnings;useCarpqw/croak/;+useFile::Tempqw/tempfile/;useIO::Fileqw//;useDigest::MD5;
@@ -2683,14 +2686,8 @@ sub apply_textdelta {my$base=IO::File->new_tmpfile;$base->autoflush(1);if($fb->{blob}){-defined(my$pid=fork)orcroak$!;-if(!$pid){-openSTDOUT,'>&',$baseorcroak$!;-printSTDOUT'link 'if($fb->{mode_a}==120000);-execqw/git-cat-file blob/,$fb->{blob}orcroak$!;-}-waitpid$pid,0;-croak$?if$?;+my$contents=$::_repository->cat_blob($fb->{blob});+print$base$contents;if(defined$exp){seek$base,0,0orcroak$!;
@@ -2729,14 +2726,18 @@ sub close_file {$bufeq'link 'ordie"$path has mode 120000","but is not a link\n";}-defined(my$pid=openmy$out,'-|')ordie"Can't fork: $!\n";-if(!$pid){-openSTDIN,'<&',$fhorcroak$!;-execqw/git-hash-object -w --stdin/orcroak$!;++my($tmp_fh,$tmp_filename)=File::Temp::tempfile(UNLINK=>1);+my$result;+while($result=sysread($fh,my$string,1024)){+syswrite($tmp_fh,$string,$result);}-chomp($hash=do{local$/;<$out>});-close$outorcroak$!;+defined$resultorcroak$!;+close$tmp_fhorcroak$!;+close$fhorcroak$!;++$hash=$::_repository->hash_and_insert_object($tmp_filename);$hash=~ /^[a-f\d]{40}$/ordie"not a sha1: $hash\n";close$fb->{base}orcroak$!;}else{
@@ -3063,13 +3064,8 @@ sub chg_file {}elsif($m->{mode_a}=~ /^120/&&$m->{mode_b}!~/^120/){$self->change_file_prop($fbat,'svn:special',undef);}-defined(my$pid=fork)orcroak$!;-if(!$pid){-openSTDOUT,'>&',$fhorcroak$!;-execqw/git-cat-file blob/,$m->{sha1_b}orcroak$!;-}-waitpid$pid,0;-croak$?if$?;+my$blob=$::_repository->cat_blob($m->{sha1_b});+print$fh$blob;$fh->flush==0orcroak$!;seek$fh,0,0orcroak$!;
@@ -8,7 +8,7 @@ git-hash-object - Compute object ID and optionally creates a blob from a file SYNOPSIS ---------'git-hash-object' [-t <type>] [-w] [--stdin] [--] <file>...+'git-hash-object' [-t <type>] [-w] [--stdin | --stdin-paths] [--] <file>... DESCRIPTION -----------
@@ -32,6 +32,9 @@ OPTIONS --stdin:: Read the object from standard input instead of from a file.+--stdin-paths::+ Read file names from stdin instead of from the command-line.+ Author ------ Written by Junio C Hamano <junkio@cox.net>
@@ -20,6 +20,7 @@ static void hash_object(const char *path, enum object_type type, int write_objec?"Unable to add %s to database":"Unable to hash %s",path);printf("%s\n",sha1_to_hex(sha1));+maybe_flush_or_die(stdout,"hash to stdout");}staticvoidhash_stdin(constchar*type,intwrite_object)
@@ -41,6 +42,7 @@ int main(int argc, char **argv)constchar*prefix=NULL;intprefix_length=-1;intno_more_flags=0;+intfound_stdin_flag=0;for(i=1;i<argc;i++){if(!no_more_flags&&argv[i][0]=='-'){
@@ -62,7 +64,32 @@ int main(int argc, char **argv)}elseif(!strcmp(argv[i],"--help"))usage(hash_object_usage);+elseif(!strcmp(argv[i],"--stdin-paths")){+structstrbufbuf,nbuf;++if(found_stdin_flag)+die("Can't use both --stdin and --stdin-paths");+found_stdin_flag=1;++strbuf_init(&buf,0);+strbuf_init(&nbuf,0);+while(strbuf_getline(&buf,stdin,'\n')!=EOF){+if(buf.buf[0]=='"'){+strbuf_reset(&nbuf);+if(unquote_c_style(&nbuf,buf.buf,NULL))+die("line is badly quoted");+strbuf_swap(&buf,&nbuf);+}+hash_object(buf.buf,type_from_string(type),write_object);+}+strbuf_release(&buf);+strbuf_release(&nbuf);+}elseif(!strcmp(argv[i],"--stdin")){+if(found_stdin_flag)+die("Can't use both --stdin and --stdin-paths");+found_stdin_flag=1;+hash_stdin(type,write_object);}else
From: Adam Roben <hidden> Date: 2016-06-15 22:43:44
These functions are more efficient ways of executing `git hash-object -w` and
`git cat-file blob` when you are dealing with many files/objects.
Signed-off-by: Adam Roben <redacted>
---
Eric Wong wrote:
quoted
+package Git::Commands;
Can this be a separate file, or a part of Git.pm? I'm sure other
scripts can eventually use this and I've been meaning to split
git-svn.perl into separate files so it's easier to follow.
I ended up making it part of Git.pm, because I realized that made far more
sense than splitting it into a separate file.
perl/Git.pm | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 95 insertions(+), 2 deletions(-)
@@ -218,7 +221,6 @@ sub repository {bless$self,$class;}-=back=head1METHODS
@@ -675,6 +677,93 @@ sub hash_object {}+=itemhash_and_insert_object(FILENAME)++ComputetheSHA1objectidofthegivenC<FILENAME>andaddtheobjecttothe+objectdatabase.++ThefunctionreturnstheSHA1hash.++=cut++# TODO: Support for passing FILEHANDLE instead of FILENAME+subhash_and_insert_object{+my($self,$filename)=@_;++$self->_open_hash_and_insert_object_if_needed();+my($in,$out)=($self->{hash_object_in},$self->{hash_object_out});++print$out$filename,"\n";+chomp(my$hash=<$in>);+return$hash;+}++sub_open_hash_and_insert_object_if_needed{+my($self)=@_;++returnifdefined($self->{hash_object_pid});++($self->{hash_object_pid},$self->{hash_object_in},+$self->{hash_object_out},$self->{hash_object_ctx})=+command_bidi_pipe(qw(hash-object-w--stdin-paths));+}++sub_close_hash_and_insert_object{+my($self)=@_;++returnunlessdefined($self->{hash_object_pid});++my@vars=map{'hash_object'.$_}qw(pidinoutctx);++command_close_bidi_pipe($self->{@vars});+delete$self->{@vars};+}++=itemcat_blob(SHA1)++ReturnsthecontentsoftheblobidentifiedbyC<SHA1>.++=cut++subcat_blob{+my($self,$sha1)=@_;++$self->_open_cat_blob_if_needed();+my($in,$out)=($self->{cat_blob_in},$self->{cat_blob_out});++print$out$sha1,"\n";+chomp(my$size=<$in>);++my$blob;+my$result=read($in,$blob,$size);+defined$resultorcarp$!;++# Skip past the trailing newline.+read($in,my$newline,1);++return$blob;+}++sub_open_cat_blob_if_needed{+my($self)=@_;++returnifdefined($self->{cat_blob_pid});++($self->{cat_blob_pid},$self->{cat_blob_in},+$self->{cat_blob_out},$self->{cat_blob_ctx})=+command_bidi_pipe(qw(cat-fileblob--stdin));+}++sub_close_cat_blob{+my($self)=@_;++returnunlessdefined($self->{cat_blob_pid});++my@vars=map{'cat_blob'.$_}qw(pidinoutctx);++command_close_bidi_pipe($self->{@vars});+delete$self->{@vars};+}=back
@@ -892,7 +981,11 @@ sub _cmd_close {}-subDESTROY{}+subDESTROY{+my($self)=@_;+$self->_close_hash_and_insert_object();+$self->_close_cat_blob();+}# Pipe implementation for ActiveState Perl.
From: Eric Wong <hidden> Date: 2016-06-15 22:43:44
Adam Roben [off-list ref] wrote:
These functions are more efficient ways of executing `git hash-object -w` and
`git cat-file blob` when you are dealing with many files/objects.
Signed-off-by: Adam Roben <redacted>
---
Eric Wong wrote:
quoted
quoted
+package Git::Commands;
Can this be a separate file, or a part of Git.pm? I'm sure other
scripts can eventually use this and I've been meaning to split
git-svn.perl into separate files so it's easier to follow.
I ended up making it part of Git.pm, because I realized that made far more
sense than splitting it into a separate file.
perl/Git.pm | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 95 insertions(+), 2 deletions(-)
I missed this the first time around. But I'd rather be able to pass a
file handle to cat_blob for writing, instead of returning a potentially
huge string in memory.
quoted hunk
@@ -675,6 +677,93 @@ sub hash_object { }+=item hash_and_insert_object ( FILENAME )++Compute the SHA1 object id of the given C<FILENAME> and add the object to the+object database.++The function returns the SHA1 hash.++=cut++# TODO: Support for passing FILEHANDLE instead of FILENAME
Filenames are fine for this input since they (are/should be) generated
by File::Temp and not from an untrusted repo.
We should, however assert that the caller of this function
isn't using a stupid filename with "\n" in it.
+=item cat_blob ( SHA1 )
+
+Returns the contents of the blob identified by C<SHA1>.
+
+=cut
+
+sub cat_blob {
+ my ($self, $sha1) = @_;
+
+ $self->_open_cat_blob_if_needed();
+ my ($in, $out) = ($self->{cat_blob_in}, $self->{cat_blob_out});
+
+ print $out $sha1, "\n";
+ chomp(my $size = <$in>);
+
+ my $blob;
+ my $result = read($in, $blob, $size);
+ defined $result or carp $!;
+
+ # Skip past the trailing newline.
+ read($in, my $newline, 1);
+
+ return $blob;
+}
However, I'd very much like to be able to pass a file handle to this
function. This should read()/print() to a file handle passed to it in a
loop rather than slurping all of $size at once, since the files we're
receiving can be huge.
I'd also be happier if we checked that we actually read $size bytes in
the loop, and that $newline is actually "\n" to safeguard against bugs
in cat-blob.
+sub _open_cat_blob_if_needed {
+ my ($self) = @_;
+
+ return if defined($self->{cat_blob_pid});
+
+ ($self->{cat_blob_pid}, $self->{cat_blob_in},
+ $self->{cat_blob_out}, $self->{cat_blob_ctx}) =
+ command_bidi_pipe(qw(cat-file blob --stdin));
+}
+
+sub _close_cat_blob {
+ my ($self) = @_;
+
+ return unless defined($self->{cat_blob_pid});
+
+ my @vars = map { 'cat_blob' . $_ } qw(pid in out ctx);
One more nit, I'm a bit paranoid, but I personally like to die/croak if
the result of every print()/syswrite() to make sure the pipe we're
writing to didn't die or if there were other error indicators.
Hopefully that's the last of tweaks I'd like to see :)
--
Eric Wong
From: Junio C Hamano <hidden> Date: 2016-06-15 22:43:44
Adam Roben [off-list ref] writes:
quoted hunk
@@ -23,6 +23,10 @@ OPTIONS For a more complete list of ways to spell object names, see "SPECIFYING REVISIONS" section in gitlink:git-rev-parse[1].+--stdin::+ Read object names from stdin instead of specifying one on the+ command line.+
This does not talk about modified output format: what the format
is, nor when that modified format is used.
quoted hunk
@@ -139,16 +139,26 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name) if (!buf) die("git-cat-file %s: bad file", obj_name);+ if (print_size) {+ printf("%lu\n", size);+ fflush(stdout);+ } write_or_die(1, buf, size);+ if (print_size) {+ printf("\n");+ fflush(stdout);+ } return 0; }
Not that I object strongly to it, but do we need extra LF after
the contents?
- "It would help readers written in typical scripting
languages" is an acceptable answer, but I doubt that is the
case --- the reader is given the number of bytes and is
going to "read($pipe, $buf, $that_size)" anyway.
- "The reader can assert that one-byte past the content is a
LF to catch errors, and this LF would help re-synchronize
after such an error" would be another acceptable answer, but
for the re-synchronization to work, the output needs to tell
which record each chunk is about (i.e. if the output were
"<type> <sha1> <size>LF<contents>LF", the "re-sync" argument
would make a bit more sense).
+ print_size = !opt || opt == 'p';
Needs a bit of comment here, and in the documentation. E.g.
git-cat-file --stdin -t <list-of-sha1
git-cat-file --stdin -s <list-of-sha1
are ways to check types and sizes of the objects in the
list.
How does --stdin interact with -e?
How does --stdin interact with -p when printing a tree or a tag
object?
How does "blob --stdin" do when input sequence contains a non
blob SHA1?
It almost feels that --stdin should be named something else,
such as --batch or --bulk, as it is not just affecting the
input.
Here is an alternative suggestion.
Two new options, --batch and --batch-check, are introduced.
These options are incompatible with -[tsep] or an object type
given as the first parameter to git-cat-file.
* git-cat-file --batch-check <list-of-sha1
outputs a record of this form
<sha1> SP <type> SP <size> LF
for each of the input lines.
* git-cat-file --batch <list-of-sha1
outputs a record of this form
<sha1> SP <type> SP <size> LF <contents> LF
for each of the input lines.
For a missing object, either option gives a record of form:
<sha1> SP missing LF
From: Junio C Hamano <hidden> Date: 2016-06-15 22:43:44
Adam Roben [off-list ref] writes:
This allows multiple paths to be specified on stdin.
Ok. List of paths is certainly a good thing to have.
In addition, if you are enhancing cat-file to spew chunked
output out, I suspect that there should be a mode of operation
for hash-object that eats that data format. IOW, this pipe
git-cat-file --batch <list-of-sha1 |
git-hash-object --batch
should be an intuitive no-op, shouldn't it?
From: Brian Downing <hidden> Date: 2016-06-15 22:43:44
On Fri, Oct 26, 2007 at 02:00:47PM -0700, Junio C Hamano wrote:
In addition, if you are enhancing cat-file to spew chunked
output out, I suspect that there should be a mode of operation
for hash-object that eats that data format. IOW, this pipe
git-cat-file --batch <list-of-sha1 |
git-hash-object --batch
should be an intuitive no-op, shouldn't it?
I think that's an obviously good thing to do. However, given your
suggested output format (which I also like):
* git-cat-file --batch <list-of-sha1
outputs a record of this form
<sha1> SP <type> SP <size> LF <contents> LF
for each of the input lines.
What should the input behavior be? Obviously the sha1 will probably
not be known on the input side. Should that simply be optional (i.e.
it will accept either "<sha1> SP <type> SP <size>" or "<type> SP <size>"
or should it only accept the latter, and a dummy sha1 will need to be
filled in if the sha1 is not known (presumably "000...000")?
-bcd