Skip to content

Add support to develop Git in Visual Studio Code#2

Closed
dscho wants to merge 9 commits intogitgitgadget:masterfrom
dscho:vscode
Closed

Add support to develop Git in Visual Studio Code#2
dscho wants to merge 9 commits intogitgitgadget:masterfrom
dscho:vscode

Conversation

@dscho
Copy link
Copy Markdown
Member

@dscho dscho commented Jun 7, 2018

Visual Studio Code (nickname "VS Code") is a light-weight, cross-platform, Open Source development environment, with an increasingly powerful extension to support C/C++ development. In particular the intellisense support as well as all the other niceties developers might have come to expect from Integrated Development Environments will help accelerate development.

Due to the way Git's source code and build process is structured, it can be quite challenging to use VS Code effectively for developing Git. Which is a shame, as developing with vim and the command-line causes unnecessary churn, and it is quite understandable that most Git developers simply do not want to fight with a modern development environment just to try whether they like developing Git with it.

This topic branch makes it easy to get started using VS Code to develop Git.

Simply run the script ./contrib/vscode/init.sh. This will initialize the .vscode/ directory and some files in that directory. After that, just open Git's top-level directory as "folder" in VS Code.

The files have to be generated because of the curious way Git determines what flags to pass to the C compiler, in particular which constants are defined, because they change the compile flow in rather dramatic ways (determining, e.g. which SHA-1 backend to use).

Changes since v1:

  • Clarified commit message of the first commit.

@dscho dscho requested a review from telezhnaya June 7, 2018 07:30
@dscho
Copy link
Copy Markdown
Member Author

dscho commented Jun 7, 2018

@telezhnaya you asked how I got started with this... so let's see what I can remember, as it has been quite a while that I first looked into using VS Code for Git development.

I got interested in this project when I read that there was a C/C++ extension for VS Code. So basically I tried all the things that were documented on that page, including the use of Intellisense.

It was quite a rough start because I had no prior experience with VS Code, so I learned on the go how to operate it, what its ideas are, and what the useful keyboard shortcuts are. My favorite shortcuts:

  • Ctrl+P to open a file in the worktree
  • Ctrl+Shift+P to open a file in the worktree
  • Ctrl+. to show auto-fix solutions for problems (just direct your cursor to the code underlined by a "squiggly line" and press that shortcut)

I did pick up some Typescript development in VS Code in the meantime, and VS Code is simply awesome for Typescript development. So even if some of the C/C++ support is not quite where I want to see it, I know at least where it is heading.

Already very early on, I ran into substantial problems with "problems". VS Code would point out some problems with certain header files, but when I opened said header files, the problems would miraculously go away. I reported this internally, and was delighted when some VS Code developer got back to me with the news that it was fixed.

After that, I tried to figure out automatic spell correction (looking for an extension, finding the cSpell thingie, configuring it in the documented ways), and how to format code automatically in the way the core Git reviewers seem to prefer it. From years of experience on the Git mailing list, I knew what to look for: by far the most common reviewer comments point out lines that are too long: they need to be <=80 columns/line in code, and <=76 columns in commit messages. Telling contributors about whitespace is also a fun occupation of the core Git reviewers, so that was next. I did not find out how to enforce the alignment of parameters (maybe you can find something?), i.e.

if (some_function_call(...) ||
    some_other_call(...)) {
        ...
}

Note how the second line is aligned to start just after the opening parentheses on the first line? That is exactly how the core Git reviewers like their code. It can get more complicated, too:

static int this_is_my_static_function(int some_argument,
                                      int next_argument)
{
        ...
}

And of course, when you have nested parentheses, the indentation should align with the appropriate opening parenthesis.

@dscho
Copy link
Copy Markdown
Member Author

dscho commented Jun 9, 2018

@telezhnaya any feedback? 😃

Copy link
Copy Markdown

@telezhnaya telezhnaya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I am sorry for such long waiting.
I have read the code - found nothing suspicious.
I also tested the solution on Linux, and there was a problem that it was not possible to debug the solution at the first time - there wasn't ssl.h file. I solved the issue by building Git for Windows in Linux terminal, and everything works fine after it, but I guess there must be a way not to use native Linux terminal at all.
Other problem was about stdout/stderr. I can't find it anywhere, I searched through all outputs. Maybe it's just my problem, but I think it's a good idea to add information about how to find the output to the Guidance.

@telezhnaya
Copy link
Copy Markdown

And - thanks a lot for your story! I am sure I'm not the only person interested in such kind of information.

@dscho
Copy link
Copy Markdown
Member Author

dscho commented Jun 15, 2018

Other problem was about stdout/stderr.

Could you describe this problem and the ssl.h one in a bit more detail?

@dscho
Copy link
Copy Markdown
Member Author

dscho commented Jun 17, 2018

Other problem was about stdout/stderr.

Could you describe this problem and the ssl.h one in a bit more detail?

@telezhnaya I think we can actually address both issues in the script. There are settings to force internal/external terminals (which might address your stdout/stderr issue, not sure until I know more about the issue), and we can make sure that we auto-detect Ubuntu and suggest to install the packages that are suggested in https://github.com/git-for-windows/git/blob/master/CONTRIBUTING.md (unless they are already installed).

@dscho dscho force-pushed the vscode branch 3 times, most recently from 3e71fcf to 60ce1e4 Compare June 23, 2018 19:00
@dscho
Copy link
Copy Markdown
Member Author

dscho commented Jul 23, 2018

/submit

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget bot commented Jul 23, 2018

Submitted as pull.2.git.gitgitgadget@gmail.com

dscho added 9 commits July 30, 2018 16:29
VS Code is a lightweight but powerful source code editor which runs on
your desktop and is available for Windows, macOS and Linux. Among other
languages, it has support for C/C++ via an extension, which offers to
not only build and debug the code, but also Intellisense, i.e.
code-aware completion and similar niceties.

This patch adds a script that helps set up the environment to work
effectively with VS Code: simply run the Unix shell script
contrib/vscode/init.sh, which creates the relevant files, and open the
top level folder of Git's source code in VS Code.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Sadly, we do not get all of the definitions via ALL_CFLAGS. Some defines
are passed to GCC *only* when compiling specific files, such as git.o.

Let's just hard-code them into the script for the time being.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
While it is technically possible, it is confusing. Not only the user,
but also VS Code's intellisense.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This helps VS Code's intellisense to figure out that we want to include
windows.h, and that we want to define the minimum target Windows version
as Windows Vista/2008R2.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The C/C++ settings are special, as they are the only generated VS Code
configurations that *will* change over the course of Git's development,
e.g. when a new constant is defined.

Therefore, let's only update the C/C++ settings, also to prevent user
modifications from being overwritten.

Ideally, we would keep user modifications in the C/C++ settings, but
that would require parsing JSON, a task for which a Unix shell script is
distinctly unsuited. So we write out .new files instead, and warn the
user if they may want to reconcile their changes.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
When configuring VS Code as core.editor (via `code --wait`), we really
want to adhere to the Git conventions of wrapping commit messages.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This adds a couple settings for the .c/.h files so that it is easier to
conform to Git's conventions while editing the source code.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The quite useful cSpell extension allows VS Code to have "squiggly"
lines under spelling mistakes. By default, this would add too much
clutter, though, because so much of Git's source code uses words that
would trigger cSpell.

Let's add a few words to make the spell checking more useful by reducing
the number of false positives.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
By default, the cSpell extension ignores all files under .git/. That
includes, unfortunately, COMMIT_EDITMSG, i.e. commit messages. However,
spell checking is *quite* useful when writing commit messages... And
since the user hardly ever opens any file inside .git (apart from commit
messages, the config, and sometimes interactive rebase's todo lists),
there is really not much harm in *not* ignoring .git/.

The default also ignores `node_modules/`, but that does not apply to
Git, so let's skip ignoring that, too.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho
Copy link
Copy Markdown
Member Author

dscho commented Jul 30, 2018

/submit

@gitgitgadget
Copy link
Copy Markdown

gitgitgadget bot commented Jul 30, 2018

Submitted as pull.2.v2.git.gitgitgadget@gmail.com

@dscho dscho closed this Oct 26, 2018
@dscho dscho deleted the vscode branch October 26, 2018 11:30
dscho pushed a commit that referenced this pull request Nov 4, 2019
Signed-off-by: Alessandro Menti <alessandro.menti@alessandromenti.it>
dscho pushed a commit that referenced this pull request Nov 4, 2019
* 'update-italian-translation' of github.com:AlessandroMenti/git-po:
  l10n: it.po: update the Italian translation for Git 2.24.0 round #2
dscho pushed a commit that referenced this pull request Nov 4, 2019
l10n-2.24.0-rnd2

* tag 'l10n-2.24.0-rnd2' of https://github.com/git-l10n/git-po:
  l10n: zh_CN: for git v2.24.0 l10n round 1~2
  l10n: de.po: Update German translation
  l10n: sv.po: Update Swedish translation (4695t0f0u)
  l10n: bg.po: Updated Bulgarian translation (4694)
  l10n: vi(4694t): Updated translation for v2.24.0
  l10n: es: 2.24.0 round 2
  l10n: it.po: update the Italian translation for Git 2.24.0 round #2
  l10n: fr v2.24.0 rnd2
  l10n: git.pot: v2.24.0 round 2 (1 new)
  l10n: it.po: update the Italian translation for Git 2.24.0
  l10n: fr 2.24.0 rnd 1
  l10n: git.pot: v2.24.0 round 1 (35 new, 16 removed)
  l10n: bg.po: Updated Bulgarian translation (4693)
  l10n: sv.po: Update Swedish translation (4674t0f0u)
  l10n: Update Catalan translation
dscho pushed a commit that referenced this pull request Jun 18, 2020
Code cleanup.

* sg/commit-graph-cleanups:
  commit-graph: persist existence of changed-paths
  commit-graph: change test to die on parse, not load
  bloom: enforce a minimum size of 8 bytes
  commit-graph: check all leading directories in changed path Bloom filters
  commit-graph: check chunk sizes after writing
  commit-graph: simplify chunk writes into loop
  commit-graph: unify the signatures of all write_graph_chunk_*() functions
  commit-graph: place bloom_settings in context
  commit-graph: simplify write_commit_graph_file() #2
  commit-graph: simplify write_commit_graph_file() #1
  commit-graph: simplify parse_commit_graph() #2
  commit-graph: simplify parse_commit_graph() #1
  commit-graph: clean up #includes
  diff.h: drop diff_tree_oid() & friends' return value
  commit-slab: add a function to deep free entries on the slab
  commit-graph-format.txt: all multi-byte numbers are in network byte order
  commit-graph: fix parsing the Chunk Lookup table
  tree-walk.c: don't match submodule entries for 'submod/anything'
dscho pushed a commit that referenced this pull request Jun 18, 2020
Code cleanup.

* sg/commit-graph-cleanups:
  commit-graph: persist existence of changed-paths
  commit-graph: change test to die on parse, not load
  bloom: enforce a minimum size of 8 bytes
  commit-graph: check all leading directories in changed path Bloom filters
  commit-graph: check chunk sizes after writing
  commit-graph: simplify chunk writes into loop
  commit-graph: unify the signatures of all write_graph_chunk_*() functions
  commit-graph: place bloom_settings in context
  commit-graph: simplify write_commit_graph_file() #2
  commit-graph: simplify write_commit_graph_file() #1
  commit-graph: simplify parse_commit_graph() #2
  commit-graph: simplify parse_commit_graph() #1
  commit-graph: clean up #includes
  diff.h: drop diff_tree_oid() & friends' return value
  commit-slab: add a function to deep free entries on the slab
  commit-graph-format.txt: all multi-byte numbers are in network byte order
  commit-graph: fix parsing the Chunk Lookup table
  tree-walk.c: don't match submodule entries for 'submod/anything'
dscho pushed a commit that referenced this pull request Jun 18, 2020
The changed-path Bloom filter is improved using ideas from an
independent implementation.

* sg/commit-graph-cleanups:
  commit-graph: persist existence of changed-paths
  commit-graph: change test to die on parse, not load
  bloom: enforce a minimum size of 8 bytes
  commit-graph: check all leading directories in changed path Bloom filters
  commit-graph: check chunk sizes after writing
  commit-graph: simplify chunk writes into loop
  commit-graph: unify the signatures of all write_graph_chunk_*() functions
  commit-graph: place bloom_settings in context
  commit-graph: simplify write_commit_graph_file() #2
  commit-graph: simplify write_commit_graph_file() #1
  commit-graph: simplify parse_commit_graph() #2
  commit-graph: simplify parse_commit_graph() #1
  commit-graph: clean up #includes
  diff.h: drop diff_tree_oid() & friends' return value
  commit-slab: add a function to deep free entries on the slab
  commit-graph-format.txt: all multi-byte numbers are in network byte order
  commit-graph: fix parsing the Chunk Lookup table
  tree-walk.c: don't match submodule entries for 'submod/anything'
dscho pushed a commit that referenced this pull request Jun 19, 2020
The changed-path Bloom filter is improved using ideas from an
independent implementation.

* sg/commit-graph-cleanups:
  commit-graph: persist existence of changed-paths
  commit-graph: change test to die on parse, not load
  bloom: enforce a minimum size of 8 bytes
  commit-graph: check all leading directories in changed path Bloom filters
  commit-graph: check chunk sizes after writing
  commit-graph: simplify chunk writes into loop
  commit-graph: unify the signatures of all write_graph_chunk_*() functions
  commit-graph: place bloom_settings in context
  commit-graph: simplify write_commit_graph_file() #2
  commit-graph: simplify write_commit_graph_file() #1
  commit-graph: simplify parse_commit_graph() #2
  commit-graph: simplify parse_commit_graph() #1
  commit-graph: clean up #includes
  diff.h: drop diff_tree_oid() & friends' return value
  commit-slab: add a function to deep free entries on the slab
  commit-graph-format.txt: all multi-byte numbers are in network byte order
  commit-graph: fix parsing the Chunk Lookup table
  tree-walk.c: don't match submodule entries for 'submod/anything'
dscho pushed a commit that referenced this pull request Jun 19, 2020
The changed-path Bloom filter is improved using ideas from an
independent implementation.

* sg/commit-graph-cleanups:
  commit-graph: persist existence of changed-paths
  commit-graph: change test to die on parse, not load
  bloom: enforce a minimum size of 8 bytes
  commit-graph: check all leading directories in changed path Bloom filters
  commit-graph: check chunk sizes after writing
  commit-graph: simplify chunk writes into loop
  commit-graph: unify the signatures of all write_graph_chunk_*() functions
  commit-graph: place bloom_settings in context
  commit-graph: simplify write_commit_graph_file() #2
  commit-graph: simplify write_commit_graph_file() #1
  commit-graph: simplify parse_commit_graph() #2
  commit-graph: simplify parse_commit_graph() #1
  commit-graph: clean up #includes
  diff.h: drop diff_tree_oid() & friends' return value
  commit-slab: add a function to deep free entries on the slab
  commit-graph-format.txt: all multi-byte numbers are in network byte order
  commit-graph: fix parsing the Chunk Lookup table
  tree-walk.c: don't match submodule entries for 'submod/anything'
dscho pushed a commit that referenced this pull request Jun 22, 2020
- commit-graph: place bloom_settings in context
- commit-graph: simplify write_commit_graph_file() #2
- commit-graph: simplify write_commit_graph_file() #1
- commit-graph: simplify parse_commit_graph() #2
- commit-graph: simplify parse_commit_graph() #1
- commit-graph: clean up #includes
- diff.h: drop diff_tree_oid() & friends' return value
- commit-slab: add a function to deep free entries on the slab
- commit-graph-format.txt: all multi-byte numbers are in network byte order
- commit-graph: fix parsing the Chunk Lookup table
- tree-walk.c: don't match submodule entries for 'submod/anything'

The changed-path Bloom filter is improved using ideas from an
independent implementation.

* sg/commit-graph-cleanups:
  commit-graph: simplify write_commit_graph_file() #2
  commit-graph: simplify write_commit_graph_file() #1
  commit-graph: simplify parse_commit_graph() #2
  commit-graph: simplify parse_commit_graph() #1
  commit-graph: clean up #includes
  diff.h: drop diff_tree_oid() & friends' return value
  commit-slab: add a function to deep free entries on the slab
  commit-graph-format.txt: all multi-byte numbers are in network byte order
  commit-graph: fix parsing the Chunk Lookup table
  tree-walk.c: don't match submodule entries for 'submod/anything'
dscho pushed a commit that referenced this pull request Jun 22, 2020
The changed-path Bloom filter is improved using ideas from an
independent implementation.

* sg/commit-graph-cleanups:
  commit-graph: simplify write_commit_graph_file() #2
  commit-graph: simplify write_commit_graph_file() #1
  commit-graph: simplify parse_commit_graph() #2
  commit-graph: simplify parse_commit_graph() #1
  commit-graph: clean up #includes
  diff.h: drop diff_tree_oid() & friends' return value
  commit-slab: add a function to deep free entries on the slab
  commit-graph-format.txt: all multi-byte numbers are in network byte order
  commit-graph: fix parsing the Chunk Lookup table
  tree-walk.c: don't match submodule entries for 'submod/anything'
dscho pushed a commit that referenced this pull request Jun 23, 2020
The changed-path Bloom filter is improved using ideas from an
independent implementation.

* sg/commit-graph-cleanups:
  commit-graph: simplify write_commit_graph_file() #2
  commit-graph: simplify write_commit_graph_file() #1
  commit-graph: simplify parse_commit_graph() #2
  commit-graph: simplify parse_commit_graph() #1
  commit-graph: clean up #includes
  diff.h: drop diff_tree_oid() & friends' return value
  commit-slab: add a function to deep free entries on the slab
  commit-graph-format.txt: all multi-byte numbers are in network byte order
  commit-graph: fix parsing the Chunk Lookup table
  tree-walk.c: don't match submodule entries for 'submod/anything'
dscho pushed a commit that referenced this pull request Jun 24, 2020
The changed-path Bloom filter is improved using ideas from an
independent implementation.

* sg/commit-graph-cleanups:
  commit-graph: simplify write_commit_graph_file() #2
  commit-graph: simplify write_commit_graph_file() #1
  commit-graph: simplify parse_commit_graph() #2
  commit-graph: simplify parse_commit_graph() #1
  commit-graph: clean up #includes
  diff.h: drop diff_tree_oid() & friends' return value
  commit-slab: add a function to deep free entries on the slab
  commit-graph-format.txt: all multi-byte numbers are in network byte order
  commit-graph: fix parsing the Chunk Lookup table
  tree-walk.c: don't match submodule entries for 'submod/anything'
dscho pushed a commit that referenced this pull request Jun 24, 2020
The changed-path Bloom filter is improved using ideas from an
independent implementation.

* sg/commit-graph-cleanups:
  commit-graph: simplify write_commit_graph_file() #2
  commit-graph: simplify write_commit_graph_file() #1
  commit-graph: simplify parse_commit_graph() #2
  commit-graph: simplify parse_commit_graph() #1
  commit-graph: clean up #includes
  diff.h: drop diff_tree_oid() & friends' return value
  commit-slab: add a function to deep free entries on the slab
  commit-graph-format.txt: all multi-byte numbers are in network byte order
  commit-graph: fix parsing the Chunk Lookup table
  tree-walk.c: don't match submodule entries for 'submod/anything'
dscho pushed a commit that referenced this pull request Jun 25, 2020
The changed-path Bloom filter is improved using ideas from an
independent implementation.

* sg/commit-graph-cleanups:
  commit-graph: simplify write_commit_graph_file() #2
  commit-graph: simplify write_commit_graph_file() #1
  commit-graph: simplify parse_commit_graph() #2
  commit-graph: simplify parse_commit_graph() #1
  commit-graph: clean up #includes
  diff.h: drop diff_tree_oid() & friends' return value
  commit-slab: add a function to deep free entries on the slab
  commit-graph-format.txt: all multi-byte numbers are in network byte order
  commit-graph: fix parsing the Chunk Lookup table
  tree-walk.c: don't match submodule entries for 'submod/anything'
dscho pushed a commit that referenced this pull request Jun 29, 2020
The changed-path Bloom filter is improved using ideas from an
independent implementation.

Under review.
cf. <20200627155610.GN2898@szeder.dev>
cf. <20200627163335.GO2898@szeder.dev>
cf. <20200627155348.GM2898@szeder.dev>

* sg/commit-graph-cleanups:
  commit-graph: simplify write_commit_graph_file() #2
  commit-graph: simplify write_commit_graph_file() #1
  commit-graph: simplify parse_commit_graph() #2
  commit-graph: simplify parse_commit_graph() #1
  commit-graph: clean up #includes
  diff.h: drop diff_tree_oid() & friends' return value
  commit-slab: add a function to deep free entries on the slab
  commit-graph-format.txt: all multi-byte numbers are in network byte order
  commit-graph: fix parsing the Chunk Lookup table
  tree-walk.c: don't match submodule entries for 'submod/anything'
dscho pushed a commit that referenced this pull request Jul 2, 2020
The changed-path Bloom filter is improved using ideas from an
independent implementation.

Under review.
cf. <20200627155610.GN2898@szeder.dev>
cf. <20200627163335.GO2898@szeder.dev>
cf. <20200627155348.GM2898@szeder.dev>

* sg/commit-graph-cleanups:
  commit-graph: simplify write_commit_graph_file() #2
  commit-graph: simplify write_commit_graph_file() #1
  commit-graph: simplify parse_commit_graph() #2
  commit-graph: simplify parse_commit_graph() #1
  commit-graph: clean up #includes
  diff.h: drop diff_tree_oid() & friends' return value
  commit-slab: add a function to deep free entries on the slab
  commit-graph-format.txt: all multi-byte numbers are in network byte order
  commit-graph: fix parsing the Chunk Lookup table
  tree-walk.c: don't match submodule entries for 'submod/anything'
dscho pushed a commit that referenced this pull request Jul 7, 2020
Updates to the changed-paths bloom filter.

* ds/commit-graph-bloom-updates:
  commit-graph: check all leading directories in changed path Bloom filters
  revision: empty pathspecs should not use Bloom filters
  revision.c: fix whitespace
  commit-graph: check chunk sizes after writing
  commit-graph: simplify chunk writes into loop
  commit-graph: unify the signatures of all write_graph_chunk_*() functions
  commit-graph: persist existence of changed-paths
  bloom: fix logic in get_bloom_filter()
  commit-graph: change test to die on parse, not load
  commit-graph: place bloom_settings in context
  commit-graph: simplify write_commit_graph_file() #2
  commit-graph: simplify write_commit_graph_file() #1
  commit-graph: simplify parse_commit_graph() #2
  commit-graph: simplify parse_commit_graph() #1
  commit-graph: clean up #includes
  diff.h: drop diff_tree_oid() & friends' return value
  commit-slab: add a function to deep free entries on the slab
  commit-graph-format.txt: all multi-byte numbers are in network byte order
  commit-graph: fix parsing the Chunk Lookup table
  tree-walk.c: don't match submodule entries for 'submod/anything'
dscho pushed a commit that referenced this pull request Jul 7, 2020
Updates to the changed-paths bloom filter.

* ds/commit-graph-bloom-updates:
  commit-graph: check all leading directories in changed path Bloom filters
  revision: empty pathspecs should not use Bloom filters
  revision.c: fix whitespace
  commit-graph: check chunk sizes after writing
  commit-graph: simplify chunk writes into loop
  commit-graph: unify the signatures of all write_graph_chunk_*() functions
  commit-graph: persist existence of changed-paths
  bloom: fix logic in get_bloom_filter()
  commit-graph: change test to die on parse, not load
  commit-graph: place bloom_settings in context
  commit-graph: simplify write_commit_graph_file() #2
  commit-graph: simplify write_commit_graph_file() #1
  commit-graph: simplify parse_commit_graph() #2
  commit-graph: simplify parse_commit_graph() #1
  commit-graph: clean up #includes
  diff.h: drop diff_tree_oid() & friends' return value
  commit-slab: add a function to deep free entries on the slab
  commit-graph-format.txt: all multi-byte numbers are in network byte order
  commit-graph: fix parsing the Chunk Lookup table
  tree-walk.c: don't match submodule entries for 'submod/anything'
dscho pushed a commit that referenced this pull request Jul 7, 2020
The changed-path Bloom filter is improved using ideas from an
independent implementation.

* sg/commit-graph-cleanups:
  commit-graph: simplify write_commit_graph_file() #2
  commit-graph: simplify write_commit_graph_file() #1
  commit-graph: simplify parse_commit_graph() #2
  commit-graph: simplify parse_commit_graph() #1
  commit-graph: clean up #includes
  diff.h: drop diff_tree_oid() & friends' return value
  commit-slab: add a function to deep free entries on the slab
  commit-graph-format.txt: all multi-byte numbers are in network byte order
  commit-graph: fix parsing the Chunk Lookup table
  tree-walk.c: don't match submodule entries for 'submod/anything'
dscho pushed a commit that referenced this pull request Jul 9, 2020
The changed-path Bloom filter is improved using ideas from an
independent implementation.

* sg/commit-graph-cleanups:
  commit-graph: simplify write_commit_graph_file() #2
  commit-graph: simplify write_commit_graph_file() #1
  commit-graph: simplify parse_commit_graph() #2
  commit-graph: simplify parse_commit_graph() #1
  commit-graph: clean up #includes
  diff.h: drop diff_tree_oid() & friends' return value
  commit-slab: add a function to deep free entries on the slab
  commit-graph-format.txt: all multi-byte numbers are in network byte order
  commit-graph: fix parsing the Chunk Lookup table
  tree-walk.c: don't match submodule entries for 'submod/anything'
dscho pushed a commit that referenced this pull request Jul 9, 2020
The changed-path Bloom filter is improved using ideas from an
independent implementation.

* sg/commit-graph-cleanups:
  commit-graph: simplify write_commit_graph_file() #2
  commit-graph: simplify write_commit_graph_file() #1
  commit-graph: simplify parse_commit_graph() #2
  commit-graph: simplify parse_commit_graph() #1
  commit-graph: clean up #includes
  diff.h: drop diff_tree_oid() & friends' return value
  commit-slab: add a function to deep free entries on the slab
  commit-graph-format.txt: all multi-byte numbers are in network byte order
  commit-graph: fix parsing the Chunk Lookup table
  tree-walk.c: don't match submodule entries for 'submod/anything'
dscho pushed a commit that referenced this pull request Jul 10, 2020
The changed-path Bloom filter is improved using ideas from an
independent implementation.

* sg/commit-graph-cleanups:
  commit-graph: simplify write_commit_graph_file() #2
  commit-graph: simplify write_commit_graph_file() #1
  commit-graph: simplify parse_commit_graph() #2
  commit-graph: simplify parse_commit_graph() #1
  commit-graph: clean up #includes
  diff.h: drop diff_tree_oid() & friends' return value
  commit-slab: add a function to deep free entries on the slab
  commit-graph-format.txt: all multi-byte numbers are in network byte order
  commit-graph: fix parsing the Chunk Lookup table
  tree-walk.c: don't match submodule entries for 'submod/anything'
dscho pushed a commit that referenced this pull request Jul 13, 2020
The changed-path Bloom filter is improved using ideas from an
independent implementation.

* sg/commit-graph-cleanups:
  commit-graph: simplify write_commit_graph_file() #2
  commit-graph: simplify write_commit_graph_file() #1
  commit-graph: simplify parse_commit_graph() #2
  commit-graph: simplify parse_commit_graph() #1
  commit-graph: clean up #includes
  diff.h: drop diff_tree_oid() & friends' return value
  commit-slab: add a function to deep free entries on the slab
  commit-graph-format.txt: all multi-byte numbers are in network byte order
  commit-graph: fix parsing the Chunk Lookup table
  tree-walk.c: don't match submodule entries for 'submod/anything'
dscho pushed a commit that referenced this pull request Jul 14, 2020
The early part of t9100 creates an unusual "doubled" history in the
"git-svn" ref. When we get to t9100.17, it looks like this:

  $ git log --oneline --graph git-svn
  [...]
  *   efd0303 detect node change from file to directory #2
  |\
  * | 3e727c0 detect node change from file to directory #2
  |/
  *   3b00468 try a deep --rmdir with a commit
  |\
  * | b4832d8 try a deep --rmdir with a commit
  |/
  * f0d7bd5 import for git svn

Each commit we make with "git commit" is paired with one from "git svn
set-tree", with the latter as a merge of the first and its grandparent.

Later, t9100.17 wants to check that "git svn fetch" gets the same trees.
And it does, but just one copy of each. So it uses rev-list to get the
tree of each commit and pipes it to "uniq" to drop the duplicates. Our
input isn't sorted, but it will find adjacent duplicates. This works
reliably because the order of commits from rev-list always shows the
duplicates next to each other. For any one of those merges, we could
choose to show its duplicate or the grandparent first. But barring
clocks running backwards, the duplicate will always have a time equal to
or greater than the grandparent. Even if equal, we break ties by showing
the first-parent first, so the duplicates remain adjacent.

But this would break if the timestamps stopped moving in chronological
order. Normally we would rely on test_tick for this, but we have _two_
sources of time here:

  - "git commit" creates one commit based on GIT_COMMITTER_DATE (which
    respects test_tick)

  - the "svn set-tree" one is based on subversion, which does not have
    an easy way to specify a timestamp

So using test_tick actually breaks the test, because now the duplicates
are far in the past, and we'll show the grandparent before the
duplicate. And likewise, a proposed change to set GIT_COMMITTER_DATE in
all scripts will break it.

We _could_ fix this by sorting before removing duplicates, but
presumably it's a useful part of the test to make sure the trees appear
in the same order in both spots. Likewise, we could use something like:

  perl -ne 'print unless $seen{$_}++'

to remove duplicates without impacting the order. But that doesn't work
either, because there are actually multiple (non-duplicate) commits with
the same trees (we change a file mode and then change it back). So we'd
actually have to de-duplicate the combination of subject and tree. Which
then further throws off t9100.18, which compares the tree hashes
exactly; we'd have to strip the result back down.

Since this test _isn't_ buggy, the simplest thing is to just work around
the proposed change by documenting our expectation that git-created
commits are correctly interleaved using the current time.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
@derrickstolee derrickstolee mentioned this pull request Jan 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants