forked from git-for-windows/git
-
Notifications
You must be signed in to change notification settings - Fork 97
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Trace2: A few final fixups from upstream/master targeting 2.22.0-RC #139
Merged
jeffhostetler
merged 4 commits into
microsoft:vfs-2.21.0
from
jeffhostetler:jh-vfs221-fixups
May 24, 2019
Merged
Trace2: A few final fixups from upstream/master targeting 2.22.0-RC #139
jeffhostetler
merged 4 commits into
microsoft:vfs-2.21.0
from
jeffhostetler:jh-vfs221-fixups
May 24, 2019
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fix trace2_data_json_fl() to check for the presence of pfn_data_json_fl in its targets, rather than pfn_data_fl, which is not actually called. Signed-off-by: Josh Steadmon <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
Fix a trivial bug that's been here since the shared/do_write_index tracing was added in 42fee7a ("trace2:data: add trace2 instrumentation to index read/write", 2019-02-22). We should have enter/leave points, not two enter/enter points. This resulted in an "enter" event without a corresponding "leave" event. Signed-off-by: Ævar Arnfjörð Bjarmason <[email protected]> Acked-by: Derrick Stolee <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
Some compilers don't allow NULL to be passed for a va_list, and e.g. "gcc (Raspbian 6.3.0-18+rpi1+deb9u1) 6.3.0 20170516" errors out like this: trace2/tr2_tgt_event.c:193:18: error: invalid operands to binary && (have ‘int’ and ‘va_list {aka __va_list}’) if (fmt && *fmt && ap) { ^^ I couldn't find any hints that va_list and pointers can be mixed, and no hints that they can't either. Morten Welinder comments: "C99, Section 7.15, simply says that va_list "is an object type suitable for holding information needed by the macros va_start, va_end, and va_copy". So clearly not guaranteed to be mixable with pointers... The portable solution is to use "va_list" everywhere in the callchain. As a consequence, both trace2_region_enter_fl() and trace2_region_leave_fl() now take a variable argument list. Signed-off-by: Torsten Bögershausen <[email protected]> Acked-by: Jeff Hostetler <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
NOTE: This commit was modified when merged into the vfs-2.21 series NOTE: to account for programdata config in addition to etc/gitconfig. Teach do_git_config_sequence() to optionally gently check for access to the system config. Use this option in read_very_early_config() when initializing trace2. In [1] SZEDER Gábor reported that my changes in [2] introduced a regression when the user does not have permission to read the system config. This commit addresses that problem by optionally ignoring that error. [1] https://public-inbox.org/git/285beb2b2d740ce20fdd8af1becf371ab39703db.1554995916.git.gitgitgadget@gmail.com/T/#m342e839289aec515523a98b5e34d7f42d3f1fd79 [2] https://public-inbox.org/git/285beb2b2d740ce20fdd8af1becf371ab39703db.1554995916.git.gitgitgadget@gmail.com/T/#m11b59c9228c698442f750ee8f9b10c629399ae48 Signed-off-by: Jeff Hostetler <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
dscho
approved these changes
May 23, 2019
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good to me!
This was referenced Jun 10, 2019
derrickstolee
added a commit
to microsoft/VFSForGit
that referenced
this pull request
Jun 12, 2019
Updates to v2.22.0.vfs.1 as discussed in microsoft/git#140. Includes these changes in microsoft/git: * microsoft/git#133 * microsoft/git#139 * microsoft/git#141 * microsoft/git#143 Also, the `post-indexchanged` hook was renamed to `post-index-change` as it was upstreamed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Here are 4 commits that were added to upstream/master after I pulled the last round
of trace2 changes from upstream. These are not essential, but may prevent a few compiler
or runtime problems.
These were taken from the following gitster branches.
jh/trace2:
22a7338 trace2: fix incorrect function pointer check
jh/trace2 and ab/trace2-typofix:
c173542 trace2: fix up a missing "leave" entry point
tb/trace2-va-list-fix:
ad006fe trace2: NULL is not allowed for va_list
jh/trace2-sid-fix:
f672dee trace2: fixup access problem on /etc/gitconfig in read_very_early_config (modified on merge)
I did not bring over the gitster/js/trace2-to-directory branch because we don't
need it with our usage pattern.