Re: [PATCH 0/8] object_id part 2
On Sat, Jun 13, 2015 at 10:45:29AM +0200, Michael Haggerty wrote: In the same email where I made those design suggestions, I also I pointed out a bug in the implementation of parse_oid_hex(). Maybe that is the reason for the test failures. It probably is, although I either botched a conversion in the branch I'm working on as well, or the area for that code is lacking tests, since I don't think the tests are failing there. In the latter case, I'll send a testsuite patch as part of my next series. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: [PATCH 0/8] object_id part 2
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 06/13/2015 12:27 AM, brian m. carlson wrote: On Fri, Jun 12, 2015 at 03:14:25PM -0700, Junio C Hamano wrote: brian m. carlson sand...@crustytoothpaste.net writes: While I did run the tests between each commit, I hadn't noticed they were failing because I don't have Apache installed on my laptop, so they were silently skipped. I'll resubmit with that fixed. It is somewhat strange that _only_ http part had failures like this, and is unnerving, too, given that a few people seem to have given at least a cursory read over the patches and didn't spot anything obviously wrong. Was that because there was a single manual botch, or was that merely that other parts of the system do not have sufficient test coverage? It appears that I broke the change in parse_fetch: convert to use struct object_id which modifies remote-curl.c, so I think it's a single manual botch. I'm going to rework that patch anyway since Michael said that he didn't like the idea of parse_oid_hex as it stands, so it will end up being mostly moot. In the same email where I made those design suggestions, I also I pointed out a bug in the implementation of parse_oid_hex(). Maybe that is the reason for the test failures. Michael - -- Michael Haggerty mhag...@alum.mit.edu -BEGIN PGP SIGNATURE- Version: GnuPG v2 iEYEARECAAYFAlV77aYACgkQwg9mrRwfmAnCyQCeIp9aSOTtQ1ABpiSybcFQFP87 fNwAoLNhQtyFQ2/fqQIvzl1gGEEOlWxa =gI7/ -END PGP SIGNATURE- -- 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 0/8] object_id part 2
On Fri, Jun 12, 2015 at 03:14:25PM -0700, Junio C Hamano wrote: brian m. carlson sand...@crustytoothpaste.net writes: While I did run the tests between each commit, I hadn't noticed they were failing because I don't have Apache installed on my laptop, so they were silently skipped. I'll resubmit with that fixed. It is somewhat strange that _only_ http part had failures like this, and is unnerving, too, given that a few people seem to have given at least a cursory read over the patches and didn't spot anything obviously wrong. Was that because there was a single manual botch, or was that merely that other parts of the system do not have sufficient test coverage? It appears that I broke the change in parse_fetch: convert to use struct object_id which modifies remote-curl.c, so I think it's a single manual botch. I'm going to rework that patch anyway since Michael said that he didn't like the idea of parse_oid_hex as it stands, so it will end up being mostly moot. I mostly introduced it in an effort to avoid having to use GIT_SHA1_HEXSZ repeatedly throughout small blocks of code, but since we don't use assignments in conditionals, it doesn't seem useful as it currently stands. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: [PATCH 0/8] object_id part 2
brian m. carlson sand...@crustytoothpaste.net writes: While I did run the tests between each commit, I hadn't noticed they were failing because I don't have Apache installed on my laptop, so they were silently skipped. I'll resubmit with that fixed. It is somewhat strange that _only_ http part had failures like this, and is unnerving, too, given that a few people seem to have given at least a cursory read over the patches and didn't spot anything obviously wrong. Was that because there was a single manual botch, or was that merely that other parts of the system do not have sufficient test coverage? -- 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 0/8] object_id part 2
On Thu, Jun 11, 2015 at 01:00:04PM -0700, Junio C Hamano wrote: Fetched that branch, built and found out that it does not pass the tests, at least these (there may be others I do not usually run that are broken by this series; I dunno), so I'll discard what I fetched for now X-. Test Summary Report --- t5540-http-push-webdav.sh (Wstat: 256 Tests: 19 Failed: 12) Failed tests: 4-10, 12-15, 17 Non-zero exit status: 1 t5539-fetch-http-shallow.sh (Wstat: 256 Tests: 3 Failed: 2) Failed tests: 2-3 Non-zero exit status: 1 t5541-http-push-smart.sh (Wstat: 256 Tests: 34 Failed: 27) Failed tests: 3-17, 22-29, 31-34 Non-zero exit status: 1 t5551-http-fetch-smart.sh (Wstat: 256 Tests: 26 Failed: 17) Failed tests: 4-14, 16, 19-20, 22, 24-25 Non-zero exit status: 1 t5550-http-fetch-dumb.sh (Wstat: 256 Tests: 29 Failed: 12) Failed tests: 3, 7-16, 19 Non-zero exit status: 1 While I did run the tests between each commit, I hadn't noticed they were failing because I don't have Apache installed on my laptop, so they were silently skipped. I'll resubmit with that fixed. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: [PATCH 0/8] object_id part 2
Junio C Hamano gits...@pobox.com writes: brian m. carlson sand...@crustytoothpaste.net writes: On Wed, Jun 10, 2015 at 11:51:14PM +, brian m. carlson wrote: On Wed, Jun 10, 2015 at 03:50:32PM -0700, Junio C Hamano wrote: brian m. carlson sand...@crustytoothpaste.net writes: Convert struct object to object_id It seems that the last one didn't make it... It appears the mail was too large for vger. Unfortunately for bisectability reasons, it is necessarily large. I'll resubmit the patch with less context. Unfortunately, the only patch I can generate that falls under to 100 KB limit is with -U0, which isn't very useful. How do you want to proceed? The branch is available at [0], or I can send the -U0 patch, or I can split it into unbisectable pieces. [0] https://github.com/bk2204/git.git object-id-part2 No approach other than just letting reviewers fetch from there and taking a look is reasonable, I would think. Fetched that branch, built and found out that it does not pass the tests, at least these (there may be others I do not usually run that are broken by this series; I dunno), so I'll discard what I fetched for now X-. Test Summary Report --- t5540-http-push-webdav.sh (Wstat: 256 Tests: 19 Failed: 12) Failed tests: 4-10, 12-15, 17 Non-zero exit status: 1 t5539-fetch-http-shallow.sh (Wstat: 256 Tests: 3 Failed: 2) Failed tests: 2-3 Non-zero exit status: 1 t5541-http-push-smart.sh (Wstat: 256 Tests: 34 Failed: 27) Failed tests: 3-17, 22-29, 31-34 Non-zero exit status: 1 t5551-http-fetch-smart.sh (Wstat: 256 Tests: 26 Failed: 17) Failed tests: 4-14, 16, 19-20, 22, 24-25 Non-zero exit status: 1 t5550-http-fetch-dumb.sh (Wstat: 256 Tests: 29 Failed: 12) Failed tests: 3, 7-16, 19 Non-zero exit status: 1 -- 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 0/8] object_id part 2
brian m. carlson sand...@crustytoothpaste.net writes: The final piece in this series is the conversion of struct object to use struct object_id. This is a necessarily large patch because of the large number of places this code is used. brian m. carlson (8): refs: convert some internal functions to use object_id sha1_file: introduce has_object_file helper. Convert struct ref to use object_id. Add a utility function to make parsing hex values easier. add_sought_entry_mem: convert to struct object_id parse_fetch: convert to use struct object_id ref_newer: convert to use struct object_id Convert struct object to object_id It seems that the last one didn't make it... -- 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 0/8] object_id part 2
brian m. carlson sand...@crustytoothpaste.net writes: On Wed, Jun 10, 2015 at 11:51:14PM +, brian m. carlson wrote: On Wed, Jun 10, 2015 at 03:50:32PM -0700, Junio C Hamano wrote: brian m. carlson sand...@crustytoothpaste.net writes: Convert struct object to object_id It seems that the last one didn't make it... It appears the mail was too large for vger. Unfortunately for bisectability reasons, it is necessarily large. I'll resubmit the patch with less context. Unfortunately, the only patch I can generate that falls under to 100 KB limit is with -U0, which isn't very useful. How do you want to proceed? The branch is available at [0], or I can send the -U0 patch, or I can split it into unbisectable pieces. [0] https://github.com/bk2204/git.git object-id-part2 No approach other than just letting reviewers fetch from there and taking a look is reasonable, I would think. Did you create this manually, or is it a mechanical scripted rewrite followed by manual clean-up? If the latter, it may help people by posting the mechanical recipe _and_ a patch that shows the manual clean-up. That is something we can reasonably review and discuss. -- 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 0/8] object_id part 2
On Wed, Jun 10, 2015 at 11:51:14PM +, brian m. carlson wrote: On Wed, Jun 10, 2015 at 03:50:32PM -0700, Junio C Hamano wrote: brian m. carlson sand...@crustytoothpaste.net writes: Convert struct object to object_id It seems that the last one didn't make it... It appears the mail was too large for vger. Unfortunately for bisectability reasons, it is necessarily large. I'll resubmit the patch with less context. Unfortunately, the only patch I can generate that falls under to 100 KB limit is with -U0, which isn't very useful. How do you want to proceed? The branch is available at [0], or I can send the -U0 patch, or I can split it into unbisectable pieces. [0] https://github.com/bk2204/git.git object-id-part2 -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: [PATCH 0/8] object_id part 2
On Wed, Jun 10, 2015 at 03:50:32PM -0700, Junio C Hamano wrote: brian m. carlson sand...@crustytoothpaste.net writes: Convert struct object to object_id It seems that the last one didn't make it... It appears the mail was too large for vger. Unfortunately for bisectability reasons, it is necessarily large. I'll resubmit the patch with less context. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: [PATCH 0/8] object_id part 2
On Wed, Jun 10, 2015 at 05:21:33PM -0700, Junio C Hamano wrote:
brian m. carlson sand...@crustytoothpaste.net writes:
[0] https://github.com/bk2204/git.git object-id-part2
No approach other than just letting reviewers fetch from there and
taking a look is reasonable, I would think.
Did you create this manually, or is it a mechanical scripted rewrite
followed by manual clean-up? If the latter, it may help people by
posting the mechanical recipe _and_ a patch that shows the manual
clean-up. That is something we can reasonably review and discuss.
It's mostly manual, but some pieces (the is_null_sha1 and sha1_to_hex
conversions) were scripted using the embedded Ruby interpreter in Vim.
The script I used to do that is below.
def refactor(s)
methods = %w(is_null_sha1 sha1_to_hex).map do |m|
[m, m.sub('sha1', 'oid')]
end.to_h
s.sub(/\A(.*)(is_null_sha1|sha1_to_hex)\(([^)]+)(\.|-)sha1\)(.*)\z/) do
[$1, methods[$2], (#{$3}#{$4}oid), $5].join('')
end
end
def replace_line(method)
func = Object.method(method)
VIM::Buffer.current.line = func.call(VIM::Buffer.current.line)
end
It was invoked as replace_line(:refactor), so it basically modified each
relevant line by passing it through refactor.
Sorry for choosing a language that's less familiar to the list: if I had
known you'd want to see it, I would have used Perl.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187
signature.asc
Description: Digital signature
[PATCH 0/8] object_id part 2
This is another series of conversions to struct object_id. This series converts more of the refs code and struct object to use struct object_id. It introduces two additional helper functions. One is has_object_file, which is the equivalent of has_sha1_file. The name was chosen to be slightly more logical than has_oid_file, although it can be changed if desired. The second helper function is parse_oid_hex. It works much like get_oid_hex, but it takes an optional length argument and returns the number of bytes parsed on success (40) and 0 on error. It's designed to be more useful in conditionals and to enable us to avoid having to write GIT_SHA1_HEXSZ in favor of simply using the return value. The final piece in this series is the conversion of struct object to use struct object_id. This is a necessarily large patch because of the large number of places this code is used. brian m. carlson (8): refs: convert some internal functions to use object_id sha1_file: introduce has_object_file helper. Convert struct ref to use object_id. Add a utility function to make parsing hex values easier. add_sought_entry_mem: convert to struct object_id parse_fetch: convert to use struct object_id ref_newer: convert to use struct object_id Convert struct object to object_id archive.c| 6 +-- bisect.c | 10 ++--- branch.c | 2 +- builtin/blame.c | 46 ++-- builtin/branch.c | 2 +- builtin/checkout.c | 24 +-- builtin/clone.c | 18 builtin/commit-tree.c| 4 +- builtin/commit.c | 8 ++-- builtin/describe.c | 20 - builtin/diff-tree.c | 12 +++--- builtin/diff.c | 12 +++--- builtin/fast-export.c| 34 +++ builtin/fetch-pack.c | 14 --- builtin/fetch.c | 54 builtin/fmt-merge-msg.c | 6 +-- builtin/for-each-ref.c | 12 +++--- builtin/fsck.c | 34 +++ builtin/grep.c | 6 +-- builtin/index-pack.c | 10 ++--- builtin/log.c| 24 +-- builtin/ls-remote.c | 2 +- builtin/merge-base.c | 8 ++-- builtin/merge-tree.c | 6 +-- builtin/merge.c | 60 +-- builtin/name-rev.c | 12 +++--- builtin/notes.c | 2 +- builtin/pack-objects.c | 16 +++ builtin/receive-pack.c | 2 +- builtin/reflog.c | 4 +- builtin/remote.c | 12 +++--- builtin/replace.c| 2 +- builtin/reset.c | 6 +-- builtin/rev-list.c | 18 builtin/rev-parse.c | 4 +- builtin/shortlog.c | 2 +- builtin/show-branch.c| 8 ++-- builtin/tag.c| 4 +- builtin/unpack-objects.c | 10 ++--- bundle.c | 10 ++--- cache-tree.c | 2 +- cache.h | 12 ++ combine-diff.c | 4 +- commit.c | 32 +++--- connect.c| 2 +- decorate.c | 2 +- diff-lib.c | 2 +- fetch-pack.c | 24 +-- fsck.c | 10 ++--- hex.c| 7 http-backend.c | 2 +- http-push.c | 88 +++ http.c | 2 +- line-log.c | 6 +-- list-objects.c | 4 +- log-tree.c | 32 +++--- merge-blobs.c| 4 +- merge-recursive.c| 22 +- merge.c | 2 +- notes-merge.c| 24 +-- object.c | 8 ++-- object.h | 2 +- pack-bitmap-write.c | 16 +++ pack-bitmap.c| 34 +++ patch-ids.c | 6 +-- pretty.c | 18 refs.c | 106 +++ remote-curl.c| 21 +- remote.c | 68 +++--- remote.h | 8 ++-- revision.c | 48 ++--- send-pack.c | 16 +++ sequencer.c | 38 - server-info.c| 2 +- sha1_file.c | 5 +++ sha1_name.c | 20 - shallow.c| 6 +-- submodule.c | 8 ++-- tag.c| 10 ++--- test-match-trees.c | 2 +- transport-helper.c | 18 transport.c | 32 +++--- transport.h | 8 ++-- tree.c | 10 ++--- upload-pack.c| 26 ++-- walker.c | 18 wt-status.c | 2 +- 87 files changed, 706 insertions(+), 679 deletions(-) -- 2.4.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to
