Skip to content
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

Hot fixes from Git for Windows v2.49.0-rc0 #1867

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dscho
Copy link
Member

@dscho dscho commented Feb 27, 2025

I needed many patches to make Git for Windows v2.49.0-rc0 compile and run the many, many CI jobs successfully. These here patches even apply to upstream Git. (Technically, the Meson sorting patch is not required to compile, but it was the fall-out from many required adjustments to make the Meson jobs happy.)

cc: Patrick Steinhardt [email protected]

In 590e081 (ident: add NO_GECOS_IN_PWENT for systems without
pw_gecos in struct passwd, 2011-05-19), code was introduced to iterate
over the `gw_gecos` field; The loop variable is of type `char *`, which
assumes that `gw_gecos` is writable.

However, it is not necessarily writable (and it is a bad idea to have it
writable in the first place), so let's switch the loop variable type to
`const char *`.

Signed-off-by: Johannes Schindelin <[email protected]>
In 904339e (Introduce support for the Meson build system,
2024-12-06) the `meson.build` file was introduced, adding also a
Windows-specific list of source files. This list was obviously meant to
be sorted alphabetically, but there is one mistake. Let's fix that.

Signed-off-by: Johannes Schindelin <[email protected]>
@dscho dscho self-assigned this Feb 27, 2025
@dscho
Copy link
Member Author

dscho commented Feb 27, 2025

/submit

Copy link

gitgitgadget bot commented Feb 27, 2025

Submitted as [email protected]

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1867/dscho/g4w-hot-fixes-v1

To fetch this version to local tag pr-1867/dscho/g4w-hot-fixes-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1867/dscho/g4w-hot-fixes-v1

@@ -59,7 +59,7 @@ static struct passwd *xgetpwuid_self(int *is_bogus)

Copy link

Choose a reason for hiding this comment

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

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Johannes Schindelin via GitGitGadget" <[email protected]>
writes:

> From: Johannes Schindelin <[email protected]>
>
> In 590e081dea7c (ident: add NO_GECOS_IN_PWENT for systems without
> pw_gecos in struct passwd, 2011-05-19), code was introduced to iterate
> over the `gw_gecos` field; The loop variable is of type `char *`, which
> assumes that `gw_gecos` is writable.
>
> However, it is not necessarily writable (and it is a bad idea to have it
> writable in the first place), so let's switch the loop variable type to
> `const char *`.
>
> Signed-off-by: Johannes Schindelin <[email protected]>
> ---
>  ident.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Can we have a bit more details here (not in the commit but after the
three-dash line, meant for context) why the change from 2011 needs a
"hotfix" today?  Has something in the build environment change?  An
updated header file has changed definition for "struct passwd", or
something silly like that?  Even though POSIX.1 does not define
gecos in its struct passwd in <pwd.h>, other string members like
pw_name and pw_dir are writable, which I found funny, and makes me
puzzled why this code from 2011 needs a hotfix all of the sudden.

> diff --git a/ident.c b/ident.c
> index caf41fb2a98..967895d8850 100644
> --- a/ident.c
> +++ b/ident.c
> @@ -59,7 +59,7 @@ static struct passwd *xgetpwuid_self(int *is_bogus)
>  
>  static void copy_gecos(const struct passwd *w, struct strbuf *name)
>  {
> -	char *src;
> +	const char *src;
>  
>  	/* Traditionally GECOS field had office phone numbers etc, separated
>  	 * with commas.  Also & stands for capitalized form of the login name.

The patch text itself looks perfectly fine, so I am not opposed to
its eventual application.  Even though we do declare "src" as
non-const, we only use it to read from it, so declaring it as const
pointer is perfectly fine and more appropriate.

But I do not see any urgency relative to Git 2.48.0 (or Git 2.48.1
for that matter) to mark this as "hotfix" implying that it should be
included in 2.49-rc1 in either the patch or the proposed commit log
message.

Thanks.

@@ -1092,11 +1092,11 @@ elif host_machine.system() == 'windows'
libgit_sources += [
Copy link

Choose a reason for hiding this comment

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

On the Git mailing list, Patrick Steinhardt wrote (reply to this):

On Thu, Feb 27, 2025 at 03:44:09PM +0000, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <[email protected]>
> 
> In 904339edbd80 (Introduce support for the Meson build system,
> 2024-12-06) the `meson.build` file was introduced, adding also a
> Windows-specific list of source files. This list was obviously meant to
> be sorted alphabetically, but there is one mistake. Let's fix that.

This looks obviously good to me, thanks!

Patrick

Copy link

gitgitgadget bot commented Feb 28, 2025

User Patrick Steinhardt <[email protected]> has been added to the cc: list.

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.

1 participant