Re: [PATCH v4 00/13] New remote-hg helper

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Oct 30, 2012 at 6:20 PM, Johannes Schindelin
<Johannes.Schindelin@xxxxxx> wrote:

> P.S.: I would still recommend to have a detailed look at the 'devel'
> branch, in particular the commits starting with "fast-export: do not refer
> to non-existing marks" and ending with "t5801: skip without hg". My
> understanding is that it was completely ignored after a brief and maybe
> too-cursory look. In the least, it has a couple of lessons we learnt the
> hard way, and if git.git is dead set on duplicating the work, making these
> mistakes again could be avoided by learning from our lessons.

% g l --grep="t5801: skip without hg" devel
1e000d4 t5801: skip without hg
bee410c t5801: skip without hg
5cdc7d0 t5801: skip without hg
05b703f t5801: skip without hg
6bb8d90 t5801: skip without hg
c70b4d0 t5801: skip without hg
2f46371 t5801: skip without hg
39bc40f t5801: skip without hg
d0a618b t5801: skip without hg

% g l --grep="fast-export: do not refer" devel
d3ac32c fast-export: do not refer to non-existing marks
bdbb22f fast-export: do not refer to non-existing marks
5d99930 fast-export: do not refer to non-existing marks
381f276 fast-export: do not refer to non-existing marks
b4686c7 fast-export: do not refer to non-existing marks
e3dfe01 fast-export: do not refer to non-existing marks
c00fe59 fast-export: do not refer to non-existing marks
ce357ce fast-export: do not refer to non-existing marks
5c1c7a4 fast-export: do not refer to non-existing marks
9c827d1 fast-export: do not refer to non-existing marks

I'll assume you are referring the latest ones:

% g log --oneline --reverse d3ac32c^..1e000d4
* d3ac32c fast-export: do not refer to non-existing marks

Not needed at all.

* b013fe0 setup_revisions: remember whether a ref was positive or not
* fb89a2c fast-export: do not export negative refs
* 7655869 setup_revisions: remember whether a ref was positive or not

I've fixed this problem already.

The solution proposed in these patches is to convoluted:
1) Requires multiple unrelated changes
2) Proposes change in committish semantics

It's hard to test, because the test to check for this is not in this
patch series, and it's testing for something completely unrelated:

---
cat > expected << EOF
reset refs/heads/master
from $(git rev-parse master)

EOF

test_expect_success 'refs are updated even if no commits need to be exported' '
        git fast-export master..master > actual &&
        test_cmp expected actual
'
---

This is most certainly not what we want.

Notice that in my patch (a single patch) I added the tests at the same
time so it's clear what it's fixing, and I also added a test to the
relevant remote-helper behavior we want:

https://github.com/felipec/git/commit/76e75315bd1bd8d9d8365bb09261a745a10ceae0

* 512cb13 t5800: test pushing a new branch with old content

If this is what the patches above were trying to fix, then yes, my
patch fixes that. Also, it's tainted by changes from another patch.

* a85de2c t5800: point out that deleting branches does not work

Correct, but hardly _necessary_.

* 2412a45 transport-helper: add trailing --

No description what's the problem, or what it's trying to fix, or
tests, so it's not possible to know if this is _needed_ or not. But
probably correct.

* 026d07c remote-helper: check helper status after import/export

Again, no explanation, but the issue was already addressed:

http://article.gmane.org/gmane.comp.version-control.git/208202

The problem is minuscule, not _needed_.

* 5165e26 remote-testgit: factor out RemoteHelper class
* 049f093 git-remote-testgit: make local a function
* f835bb2 git_remote_helpers: add fastimport library
* 088ad33 git-remote-hg: add hgimport, an hg-fast-import equivalent
* 7de6ca0 git-remote-hg: add GitHg, a helper class for converting hg
commits to git
* e3cc5ed git-remote-hg: add hgexport, an hg-fast-export equivalent
* 0edc8e9 git-remote-hg: add GitExporter/GitImporter/NonLocalGit
* 5c73277 remote-hg: adjust to hg 1.9
* 1b47007 git-remote-hg: add the helper
* 4dcc671 git-remote-hg: add tests
* 48b2769 remote-hg: Postel's law dictates we should handle Author<author@mail>
* 2587cc6 remote-hg: another case of Postel's law
* 9f934c9 remote-hg: handle another funny author line from
http://scelenic.com/hg
* a799904 remote-hg: do not interfer with hg's revs() method

All these are specific to this remote-hg version.

* ac77256 Always auto-gc after calling a fast-import transport

This might be a good idea, but not _needed_.

* 1e000d4 t5801: skip without hg

Specific to this remote-hg.


So, yeah, nothing really needed there. Some patches might be nice, but
that's it.

Now, if this is really the latest and greatest remote-hg patch series,
I can try to port them to git's master and see how it fares.

But you mentioned something about cooperation, and I've yet to see how
is it that you are planning to cooperate. If you say you don't have
time to spend on this, I don't see why I should worry about testing
this series of patches.

Also, you seem to be clearly against my implementation, is there any
evidence that will convince you that my version is "good"? Maybe my
version passing more tests than msysgit's? Or is there truly nothing I
can do to change your perception?

Cheers.

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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]