| Message ID | 486297ec8c146e0ed47cd1dd8fe8f6496c2b54c2.1560860634.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix MSVC support, at long last | expand |
On Tue, Jun 18, 2019 at 8:24 AM Jeff Hostetler via GitGitGadget <gitgitgadget@gmail.com> wrote: > In MSVC, the DEBUG constant is set automatically whenever compiling with > debug information. > > This is clearly not what was intended in cache-tree.c, so let's use a less > ambiguous constant there. s/constant/macro name/ would be clearer. > Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
shouldn't this also be a problem with builtin/blame.c? Carlo
Hi Carlo,
On Tue, 18 Jun 2019, Carlo Arenas wrote:
> shouldn't this also be a problem with builtin/blame.c?
Sharp eyes!
It does not *really* matter as much here, as that file defines that
`DEBUG` constant if it has not yet been defined, but yes, it assumes that
*if* it is defined, then it is set to `1`. Which is the case, but it is
fragile.
I changed it locally already, and it will be part of the next iteration.
Thanks,
Johannes
Hi Eric, On Tue, 18 Jun 2019, Eric Sunshine wrote: > On Tue, Jun 18, 2019 at 8:24 AM Jeff Hostetler via GitGitGadget > <gitgitgadget@gmail.com> wrote: > > In MSVC, the DEBUG constant is set automatically whenever compiling with > > debug information. > > > > This is clearly not what was intended in cache-tree.c, so let's use a less > > ambiguous constant there. > > s/constant/macro name/ would be clearer. To me, "macro" always sounds as if it referred to executable code, or at least to something that expands to code. I went with s/constant/name/ instead. Thanks, Dscho > > > Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> >
Eric Sunshine <sunshine@sunshineco.com> writes: > On Tue, Jun 18, 2019 at 8:24 AM Jeff Hostetler via GitGitGadget > <gitgitgadget@gmail.com> wrote: >> In MSVC, the DEBUG constant is set automatically whenever compiling with >> debug information. >> >> This is clearly not what was intended in cache-tree.c, so let's use a less >> ambiguous constant there. > > s/constant/macro name/ would be clearer. It is closer to the standard-kosher terminology, I would think; if somebody is in more pedantic mood, "C preprocessor macro" is probably the phrase to use.
while those two changes (from DEBUG to DEBUG_$foo) are worth doing in their own merit, I am more inclined to consider this as orthogonal since by your own description[1] the right name to use would be _DEBUG (with a preceding dash) and that would obviously not conflict here. the only remaining change then would be to drop the -DDEBUG that gets added to your BASIC_CFLAGS Carlo [1] https://docs.microsoft.com/en-us/cpp/build/reference/md-mt-ld-use-run-time-library?view=vs-2019
Hi Carlo, On Wed, 19 Jun 2019, Carlo Arenas wrote: > while those two changes (from DEBUG to DEBUG_$foo) are worth doing in > their own merit, I am more inclined to consider this as orthogonal > since by your own description[1] the right name to use would be _DEBUG > (with a preceding dash) and that would obviously not conflict here. Well, both `_DEBUG` and `DEBUG` appear to be defined by Visual Studio in debug mode. So we still need to rename them, and I'd rather rename them here than in the next patch series (that will (re-)add support to build in Visual Studio), as I want the command-line MSVC build to also define both constants. > the only remaining change then would be to drop the -DDEBUG that gets > added to your BASIC_CFLAGS No, for consistency with Visual Studio, I want to keep defining both `DEBUG` and `_DEBUG`. It's just easier that way, as I won't have to spend extra cycles to remember that those two ways to build Git are slightly different, when they shouldn't be. Thanks, Dscho > Carlo > [1] https://docs.microsoft.com/en-us/cpp/build/reference/md-mt-ld-use-run-time-library?view=vs-2019 > >
diff --git a/cache-tree.c b/cache-tree.c index b13bfaf71e..706ffcf188 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -6,8 +6,8 @@ #include "object-store.h" #include "replace-object.h" -#ifndef DEBUG -#define DEBUG 0 +#ifndef DEBUG_CACHE_TREE +#define DEBUG_CACHE_TREE 0 #endif struct cache_tree *cache_tree(void) @@ -111,7 +111,7 @@ static int do_invalidate_path(struct cache_tree *it, const char *path) int namelen; struct cache_tree_sub *down; -#if DEBUG +#if DEBUG_CACHE_TREE fprintf(stderr, "cache-tree invalidate <%s>\n", path); #endif @@ -398,7 +398,7 @@ static int update_one(struct cache_tree *it, strbuf_addf(&buffer, "%o %.*s%c", mode, entlen, path + baselen, '\0'); strbuf_add(&buffer, oid->hash, the_hash_algo->rawsz); -#if DEBUG +#if DEBUG_CACHE_TREE fprintf(stderr, "cache-tree update-one %o %.*s\n", mode, entlen, path + baselen); #endif @@ -421,7 +421,7 @@ static int update_one(struct cache_tree *it, strbuf_release(&buffer); it->entry_count = to_invalidate ? -1 : i - *skip_count; -#if DEBUG +#if DEBUG_CACHE_TREE fprintf(stderr, "cache-tree update-one (%d ent, %d subtree) %s\n", it->entry_count, it->subtree_nr, oid_to_hex(&it->oid)); @@ -462,7 +462,7 @@ static void write_one(struct strbuf *buffer, struct cache_tree *it, strbuf_add(buffer, path, pathlen); strbuf_addf(buffer, "%c%d %d\n", 0, it->entry_count, it->subtree_nr); -#if DEBUG +#if DEBUG_CACHE_TREE if (0 <= it->entry_count) fprintf(stderr, "cache-tree <%.*s> (%d ent, %d subtree) %s\n", pathlen, path, it->entry_count, it->subtree_nr, @@ -536,7 +536,7 @@ static struct cache_tree *read_one(const char **buffer, unsigned long *size_p) size -= rawsz; } -#if DEBUG +#if DEBUG_CACHE_TREE if (0 <= it->entry_count) fprintf(stderr, "cache-tree <%s> (%d ent, %d subtree) %s\n", *buffer, it->entry_count, subtree_nr,