{"thread":{"id":"64446","subject":"[PATCH] wincred: align Makefile with other Makefiles in contrib","startedAt":"2025-11-05T19:55:33Z","lastAt":"2025-11-09T11:46:02Z","messageCount":7,"participants":["Thomas Uhle","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"530268","messageId":"3869ec21-e20d-cf9b-5913-6389c372a5f0@mailbox.tu-dresden.de","threadId":"64446","inReplyTo":null,"subject":"[PATCH] wincred: align Makefile with other Makefiles in contrib","fromName":"Thomas Uhle","fromEmail":"thomas.uhle@mailbox.tu-dresden.de","sentAt":"2025-11-05T19:55:19Z","receivedAt":"2025-11-05T19:55:33Z","isPatch":true,"sender":{"key":"thomas.uhle@mailbox.tu-dresden.de","avatar":"https://avatars.githubusercontent.com/u/6583219?v=4"},"body":"* Replace $(LOADLIBES) because it is deprecated since long and it is\n used nowhere else in the git project.\n* Use $(gitexecdir) instead of $(libexecdir) because config.mak defines\n $(libexecdir) as $(prefix)/libexec, not as $(prefix)/libexec/git-core.\n* Similar to other Makefiles, let install target rule create\n $(gitexecdir) to make sure the directory exists before copying the\n executable and also let it respect $(DESTDIR).\n* Shuffle the lines for the default settings to align them with the\n other Makefiles in contrib/credential.\n* Define .PHONY for all special targets (all, install, clean).\n\nSigned-off-by: Thomas Uhle \n---\n contrib/credential/wincred/Makefile | 18 ++++++++++--------\n 1 file changed, 10 insertions(+), 8 deletions(-)\n\ndiff --git a/contrib/credential/wincred/Makefile b/contrib/credential/wincred/Makefile\nindex 5b795fc..d92e721 100644\n--- a/contrib/credential/wincred/Makefile\n+++ b/contrib/credential/wincred/Makefile\n@@ -4,20 +4,22 @@\n -include ../../../config.mak.autogen\n -include ../../../config.mak\n\n-CC ?= gcc\n-RM ?= rm -f\n-CFLAGS ?= -O2 -Wall\n-\n prefix ?= /usr/local\n-libexecdir ?= $(prefix)/libexec/git-core\n+gitexecdir ?= $(prefix)/libexec/git-core\n\n+CC ?= gcc\n+CFLAGS ?= -O2 -Wall\n INSTALL ?= install\n+RM ?= rm -f\n\n-git-credential-wincred.exe : git-credential-wincred.c\n-\t$(LINK.c) $^ $(LOADLIBES) $(LDLIBS) -o $@\n+git-credential-wincred.exe: git-credential-wincred.c\n+\t$(LINK.c) -o $@ $^ $(LDFLAGS) $(LDLIBS)\n\n install: git-credential-wincred.exe\n-\t$(INSTALL) -m 755 $^ $(libexecdir)\n+\t$(INSTALL) -d -m 755 $(DESTDIR)$(gitexecdir)\n+\t$(INSTALL) -m 755 $< $(DESTDIR)$(gitexecdir)\n\n clean:\n \t$(RM) git-credential-wincred.exe\n+\n+.PHONY: all install clean\n\nbase-commit: 4cf919bd7b946477798af5414a371b23fd68bf93\n-- \n2.47.3\n"},{"id":"530319","messageId":"xmqqtsz7jkgh.fsf@gitster.g","threadId":"64446","inReplyTo":"3869ec21-e20d-cf9b-5913-6389c372a5f0@mailbox.tu-dresden.de","subject":"Re: [PATCH] wincred: align Makefile with other Makefiles in contrib","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-11-06T14:37:34Z","receivedAt":"2025-11-06T14:37:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Uhle writes:\n\n> * Replace $(LOADLIBES) because it is deprecated since long and it is\n> used nowhere else in the git project.\n> * Use $(gitexecdir) instead of $(libexecdir) because config.mak defines\n> $(libexecdir) as $(prefix)/libexec, not as $(prefix)/libexec/git-core.\n> * Similar to other Makefiles, let install target rule create\n> $(gitexecdir) to make sure the directory exists before copying the\n> executable and also let it respect $(DESTDIR).\n> * Shuffle the lines for the default settings to align them with the\n> other Makefiles in contrib/credential.\n> * Define .PHONY for all special targets (all, install, clean).\n>\n> Signed-off-by: Thomas Uhle \n> ---\n> contrib/credential/wincred/Makefile | 18 ++++++++++--------\n> 1 file changed, 10 insertions(+), 8 deletions(-)\n\nLooks sensible (to a person who does not do Windows, anyway), but I\ndo not know what depends on the way it is currently laid out, so\nwill queue only after I see a Windows person or two give their acks\nto the patch.\n\nThanks.\n\n\n\n> diff --git a/contrib/credential/wincred/Makefile b/contrib/credential/wincred/Makefile\n> index 5b795fc..d92e721 100644\n> --- a/contrib/credential/wincred/Makefile\n> +++ b/contrib/credential/wincred/Makefile\n> @@ -4,20 +4,22 @@\n> -include ../../../config.mak.autogen\n> -include ../../../config.mak\n>\n> -CC ?= gcc\n> -RM ?= rm -f\n> -CFLAGS ?= -O2 -Wall\n> -\n> prefix ?= /usr/local\n> -libexecdir ?= $(prefix)/libexec/git-core\n> +gitexecdir ?= $(prefix)/libexec/git-core\n>\n> +CC ?= gcc\n> +CFLAGS ?= -O2 -Wall\n> INSTALL ?= install\n> +RM ?= rm -f\n>\n> -git-credential-wincred.exe : git-credential-wincred.c\n> -\t$(LINK.c) $^ $(LOADLIBES) $(LDLIBS) -o $@\n> +git-credential-wincred.exe: git-credential-wincred.c\n> +\t$(LINK.c) -o $@ $^ $(LDFLAGS) $(LDLIBS)\n>\n> install: git-credential-wincred.exe\n> -\t$(INSTALL) -m 755 $^ $(libexecdir)\n> +\t$(INSTALL) -d -m 755 $(DESTDIR)$(gitexecdir)\n> +\t$(INSTALL) -m 755 $< $(DESTDIR)$(gitexecdir)\n>\n> clean:\n> \t$(RM) git-credential-wincred.exe\n> +\n> +.PHONY: all install clean\n>\n> base-commit: 4cf919bd7b946477798af5414a371b23fd68bf93\n"},{"id":"530324","messageId":"927c2094-3654-edec-072e-1f6ca7d91f96@gmx.de","threadId":"64446","inReplyTo":"3869ec21-e20d-cf9b-5913-6389c372a5f0@mailbox.tu-dresden.de","subject":"Re: [PATCH] wincred: align Makefile with other Makefiles in contrib","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2025-11-06T15:37:45Z","receivedAt":"2025-11-06T15:37:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hallo Thomas, wie geht's? Grüße an den Biergarten am Blauen Wunder!\n\nOn Wed, 5 Nov 2025, Thomas Uhle wrote:\n\n> * Replace $(LOADLIBES) because it is deprecated since long and it is\n> used nowhere else in the git project.\n> * Use $(gitexecdir) instead of $(libexecdir) because config.mak defines\n> $(libexecdir) as $(prefix)/libexec, not as $(prefix)/libexec/git-core.\n> * Similar to other Makefiles, let install target rule create\n> $(gitexecdir) to make sure the directory exists before copying the\n> executable and also let it respect $(DESTDIR).\n> * Shuffle the lines for the default settings to align them with the\n> other Makefiles in contrib/credential.\n> * Define .PHONY for all special targets (all, install, clean).\n\nThese changes all make sense to me. Feel free to add\n\n\tAcked-by: Johannes Schindelin \n\nThank you,\nJohannes\n\n> \n> Signed-off-by: Thomas Uhle \n> ---\n> contrib/credential/wincred/Makefile | 18 ++++++++++--------\n> 1 file changed, 10 insertions(+), 8 deletions(-)\n> \n> diff --git a/contrib/credential/wincred/Makefile b/contrib/credential/wincred/Makefile\n> index 5b795fc..d92e721 100644\n> --- a/contrib/credential/wincred/Makefile\n> +++ b/contrib/credential/wincred/Makefile\n> @@ -4,20 +4,22 @@\n> -include ../../../config.mak.autogen\n> -include ../../../config.mak\n> \n> -CC ?= gcc\n> -RM ?= rm -f\n> -CFLAGS ?= -O2 -Wall\n> -\n> prefix ?= /usr/local\n> -libexecdir ?= $(prefix)/libexec/git-core\n> +gitexecdir ?= $(prefix)/libexec/git-core\n> \n> +CC ?= gcc\n> +CFLAGS ?= -O2 -Wall\n> INSTALL ?= install\n> +RM ?= rm -f\n> \n> -git-credential-wincred.exe : git-credential-wincred.c\n> -\t$(LINK.c) $^ $(LOADLIBES) $(LDLIBS) -o $@\n> +git-credential-wincred.exe: git-credential-wincred.c\n> +\t$(LINK.c) -o $@ $^ $(LDFLAGS) $(LDLIBS)\n> \n> install: git-credential-wincred.exe\n> -\t$(INSTALL) -m 755 $^ $(libexecdir)\n> +\t$(INSTALL) -d -m 755 $(DESTDIR)$(gitexecdir)\n> +\t$(INSTALL) -m 755 $< $(DESTDIR)$(gitexecdir)\n> \n> clean:\n> \t$(RM) git-credential-wincred.exe\n> +\n> +.PHONY: all install clean\n> \n> base-commit: 4cf919bd7b946477798af5414a371b23fd68bf93\n> -- \n> 2.47.3\n> \n> \n"},{"id":"530326","messageId":"xmqqv7jnhzmh.fsf@gitster.g","threadId":"64446","inReplyTo":"927c2094-3654-edec-072e-1f6ca7d91f96@gmx.de","subject":"Re: [PATCH] wincred: align Makefile with other Makefiles in contrib","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-11-06T16:52:54Z","receivedAt":"2025-11-06T16:52:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Hallo Thomas, wie geht's? Grüße an den Biergarten am Blauen Wunder!\n>\n> On Wed, 5 Nov 2025, Thomas Uhle wrote:\n>\n>> * Replace $(LOADLIBES) because it is deprecated since long and it is\n>> used nowhere else in the git project.\n>> * Use $(gitexecdir) instead of $(libexecdir) because config.mak defines\n>> $(libexecdir) as $(prefix)/libexec, not as $(prefix)/libexec/git-core.\n>> * Similar to other Makefiles, let install target rule create\n>> $(gitexecdir) to make sure the directory exists before copying the\n>> executable and also let it respect $(DESTDIR).\n>> * Shuffle the lines for the default settings to align them with the\n>> other Makefiles in contrib/credential.\n>> * Define .PHONY for all special targets (all, install, clean).\n>\n> These changes all make sense to me. Feel free to add\n>\n> \tAcked-by: Johannes Schindelin \n>\n> Thank you,\n> Johannes\n\nThanks, both. Will queue with your Ack.\n\n>\n>> \n>> Signed-off-by: Thomas Uhle \n>> ---\n>> contrib/credential/wincred/Makefile | 18 ++++++++++--------\n>> 1 file changed, 10 insertions(+), 8 deletions(-)\n>> \n>> diff --git a/contrib/credential/wincred/Makefile b/contrib/credential/wincred/Makefile\n>> index 5b795fc..d92e721 100644\n>> --- a/contrib/credential/wincred/Makefile\n>> +++ b/contrib/credential/wincred/Makefile\n>> @@ -4,20 +4,22 @@\n>> -include ../../../config.mak.autogen\n>> -include ../../../config.mak\n>> \n>> -CC ?= gcc\n>> -RM ?= rm -f\n>> -CFLAGS ?= -O2 -Wall\n>> -\n>> prefix ?= /usr/local\n>> -libexecdir ?= $(prefix)/libexec/git-core\n>> +gitexecdir ?= $(prefix)/libexec/git-core\n>> \n>> +CC ?= gcc\n>> +CFLAGS ?= -O2 -Wall\n>> INSTALL ?= install\n>> +RM ?= rm -f\n>> \n>> -git-credential-wincred.exe : git-credential-wincred.c\n>> -\t$(LINK.c) $^ $(LOADLIBES) $(LDLIBS) -o $@\n>> +git-credential-wincred.exe: git-credential-wincred.c\n>> +\t$(LINK.c) -o $@ $^ $(LDFLAGS) $(LDLIBS)\n>> \n>> install: git-credential-wincred.exe\n>> -\t$(INSTALL) -m 755 $^ $(libexecdir)\n>> +\t$(INSTALL) -d -m 755 $(DESTDIR)$(gitexecdir)\n>> +\t$(INSTALL) -m 755 $< $(DESTDIR)$(gitexecdir)\n>> \n>> clean:\n>> \t$(RM) git-credential-wincred.exe\n>> +\n>> +.PHONY: all install clean\n>> \n>> base-commit: 4cf919bd7b946477798af5414a371b23fd68bf93\n>> -- \n>> 2.47.3\n>> \n>> \n"},{"id":"530362","messageId":"19573251-81e1-e07d-0f21-1f90ea5153a3@mailbox.tu-dresden.de","threadId":"64446","inReplyTo":"xmqqv7jnhzmh.fsf@gitster.g","subject":"Re: [PATCH] wincred: align Makefile with other Makefiles in contrib","fromName":"Thomas Uhle","fromEmail":"thomas.uhle@mailbox.tu-dresden.de","sentAt":"2025-11-07T11:45:33Z","receivedAt":"2025-11-07T11:45:51Z","isPatch":true,"sender":{"key":"thomas.uhle@mailbox.tu-dresden.de","avatar":"https://avatars.githubusercontent.com/u/6583219?v=4"},"body":"On Thu, 6 Nov 2025, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n>\n>> Hallo Thomas, wie geht's? Grüße an den Biergarten am Blauen Wunder!\n\nIch komme am Schillergarten so bald leider nicht vorbei, es ist einfach zu \nungemütlich draußen, und Glühwein gibt es vermutlich auch noch nicht. ;)\n\n>> On Wed, 5 Nov 2025, Thomas Uhle wrote:\n>>\n>>> * Replace $(LOADLIBES) because it is deprecated since long and it is\n>>> used nowhere else in the git project.\n>>> * Use $(gitexecdir) instead of $(libexecdir) because config.mak defines\n>>> $(libexecdir) as $(prefix)/libexec, not as $(prefix)/libexec/git-core.\n>>> * Similar to other Makefiles, let install target rule create\n>>> $(gitexecdir) to make sure the directory exists before copying the\n>>> executable and also let it respect $(DESTDIR).\n>>> * Shuffle the lines for the default settings to align them with the\n>>> other Makefiles in contrib/credential.\n>>> * Define .PHONY for all special targets (all, install, clean).\n>>\n>> These changes all make sense to me. Feel free to add\n>>\n>> \tAcked-by: Johannes Schindelin \n>>\n>> Thank you,\n>> Johannes\n>\n> Thanks, both. Will queue with your Ack.\n\nThank you! Does this patch qualify for the final version 2.52.0 or is it \nalready too late? And if it is the latter, wouldn't it make sense to have \nit in an updated version 2.52.1?\n\nBest regards,\n\nThomas\n\n\n>>>\n>>> Signed-off-by: Thomas Uhle \n>>> ---\n>>> contrib/credential/wincred/Makefile | 18 ++++++++++--------\n>>> 1 file changed, 10 insertions(+), 8 deletions(-)\n>>>\n>>> diff --git a/contrib/credential/wincred/Makefile b/contrib/credential/wincred/Makefile\n>>> index 5b795fc..d92e721 100644\n>>> --- a/contrib/credential/wincred/Makefile\n>>> +++ b/contrib/credential/wincred/Makefile\n>>> @@ -4,20 +4,22 @@\n>>> -include ../../../config.mak.autogen\n>>> -include ../../../config.mak\n>>>\n>>> -CC ?= gcc\n>>> -RM ?= rm -f\n>>> -CFLAGS ?= -O2 -Wall\n>>> -\n>>> prefix ?= /usr/local\n>>> -libexecdir ?= $(prefix)/libexec/git-core\n>>> +gitexecdir ?= $(prefix)/libexec/git-core\n>>>\n>>> +CC ?= gcc\n>>> +CFLAGS ?= -O2 -Wall\n>>> INSTALL ?= install\n>>> +RM ?= rm -f\n>>>\n>>> -git-credential-wincred.exe : git-credential-wincred.c\n>>> -\t$(LINK.c) $^ $(LOADLIBES) $(LDLIBS) -o $@\n>>> +git-credential-wincred.exe: git-credential-wincred.c\n>>> +\t$(LINK.c) -o $@ $^ $(LDFLAGS) $(LDLIBS)\n>>>\n>>> install: git-credential-wincred.exe\n>>> -\t$(INSTALL) -m 755 $^ $(libexecdir)\n>>> +\t$(INSTALL) -d -m 755 $(DESTDIR)$(gitexecdir)\n>>> +\t$(INSTALL) -m 755 $< $(DESTDIR)$(gitexecdir)\n>>>\n>>> clean:\n>>> \t$(RM) git-credential-wincred.exe\n>>> +\n>>> +.PHONY: all install clean\n>>>\n>>> base-commit: 4cf919bd7b946477798af5414a371b23fd68bf93\n>>> --\n>>> 2.47.3\n>>>"},{"id":"530384","messageId":"xmqqwm41g605.fsf@gitster.g","threadId":"64446","inReplyTo":"19573251-81e1-e07d-0f21-1f90ea5153a3@mailbox.tu-dresden.de","subject":"Re: [PATCH] wincred: align Makefile with other Makefiles in contrib","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-11-07T16:30:18Z","receivedAt":"2025-11-07T16:30:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Uhle writes:\n\n> Thank you! Does this patch qualify for the final version 2.52.0 or is it \n> already too late? And if it is the latter, wouldn't it make sense to have \n> it in an updated version 2.52.1?\n\nHighly unlikely, I would suspect.\n\nIn general, after -rc1 gets tagged, nothing will become candidate\nfor the final release without a valid excuse. One common reason is\nthat it is a bugfix for a regression that was introduced during the\ncycle. This clearly isn't one---the aspect of the wincred Makefile\nyour patch fixes haven't changed since ccfb5bda (wincred: add\ninstall target, 2012-10-24). People lived with that awkwardness for\n13 years. They can live with it a few more months just fine.\n\nThose who _have_ been building wincred and installing it for their\nown (or for their colleages) would have an established procedure to\nwork around the unusual arrangement the Makefile has (which you have\nfixed), and changing it this close to the final release would only\nadd extra work on them, without helping anybody else. A good time\nto merge such a change is early in a fresh cycle, so that they have\nlonger preparation period to adjust their build infrastructure.\n\nThere are reasons we may want to have changes newly floated after\n-rc1 got tagged; for example, I merged 8d716966 (ci: update\n{download,upload}-artifact Action versions, 2025-11-06) after\ntagging -rc1. There were another CI fix merged immediately before\n-rc1.\n\nThe benefit any late changes that get merged has to outweigh the\nrisks by a large margin, and CI changes like these have very small\nblast radius even if it goes wrong (nobody other than our developers\nwould be affected, and they know what to do) while the damage\nunfixed CI job can cause is larger (CI can deliberately stop to make\nus realize that the service we rely on is being deprecated).\n\nThere also is a message typofix merged post -rc1, to correct new\nmessages that appeared during this cycle. The output from the\nprograms before the release candidate were properly localizable, but\nleft unfixed, our translators need to translate typoed messages, and\nthen when the typofix hits 'master' later, they have to adjust their\ntranslations by updating what original gets translated again.\n\nIs there comparable justification why wincred/Makefile change has to\nbe in the upcoming release? I do not think of any.\n"},{"id":"530423","messageId":"cef2a5b6-8133-0e5a-5523-80625d9d4671@mailbox.tu-dresden.de","threadId":"64446","inReplyTo":"xmqqwm41g605.fsf@gitster.g","subject":"Re: [PATCH] wincred: align Makefile with other Makefiles in contrib","fromName":"Thomas Uhle","fromEmail":"thomas.uhle@mailbox.tu-dresden.de","sentAt":"2025-11-09T11:45:43Z","receivedAt":"2025-11-09T11:46:02Z","isPatch":true,"sender":{"key":"thomas.uhle@mailbox.tu-dresden.de","avatar":"https://avatars.githubusercontent.com/u/6583219?v=4"},"body":"On Fri, 7 Nov 2025, Junio C Hamano wrote:\n\n> Thomas Uhle writes:\n>\n>> Thank you! Does this patch qualify for the final version 2.52.0 or is it\n>> already too late? And if it is the latter, wouldn't it make sense to have\n>> it in an updated version 2.52.1?\n>\n> Highly unlikely, I would suspect.\n>\n> In general, after -rc1 gets tagged, nothing will become candidate\n> for the final release without a valid excuse. One common reason is\n> that it is a bugfix for a regression that was introduced during the\n> cycle. This clearly isn't one---the aspect of the wincred Makefile\n> your patch fixes haven't changed since ccfb5bda (wincred: add\n> install target, 2012-10-24). People lived with that awkwardness for\n> 13 years. They can live with it a few more months just fine.\n\nI agree. Fair enough.\n\n\n> Those who _have_ been building wincred and installing it for their\n> own (or for their colleages) would have an established procedure to\n> work around the unusual arrangement the Makefile has (which you have\n> fixed), and changing it this close to the final release would only\n> add extra work on them, without helping anybody else. A good time\n> to merge such a change is early in a fresh cycle, so that they have\n> longer preparation period to adjust their build infrastructure.\n\nUnderstood.\n\n\n> There are reasons we may want to have changes newly floated after\n> -rc1 got tagged; for example, I merged 8d716966 (ci: update\n> {download,upload}-artifact Action versions, 2025-11-06) after\n> tagging -rc1. There were another CI fix merged immediately before\n> -rc1.\n>\n> The benefit any late changes that get merged has to outweigh the\n> risks by a large margin, and CI changes like these have very small\n> blast radius even if it goes wrong (nobody other than our developers\n> would be affected, and they know what to do) while the damage\n> unfixed CI job can cause is larger (CI can deliberately stop to make\n> us realize that the service we rely on is being deprecated).\n>\n> There also is a message typofix merged post -rc1, to correct new\n> messages that appeared during this cycle. The output from the\n> programs before the release candidate were properly localizable, but\n> left unfixed, our translators need to translate typoed messages, and\n> then when the typofix hits 'master' later, they have to adjust their\n> translations by updating what original gets translated again.\n>\n> Is there comparable justification why wincred/Makefile change has to\n> be in the upcoming release? I do not think of any.\n\nYou are right though. I have just thought that it might be good to \nhave all the three commits for the update of the Makefiles in \ncontrib/credential \"in one go\". Yet, this is by far not comparable \nto those other cases. Thanks for your explanations!\n\nBest regards,\n\nThomas Uhle\n"}]}