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

Why remove i18n support for git? #31980

Closed
3 of 6 tasks
moonfruit opened this issue Sep 11, 2018 · 20 comments
Closed
3 of 6 tasks

Why remove i18n support for git? #31980

moonfruit opened this issue Sep 11, 2018 · 20 comments
Labels
outdated PR was locked due to age

Comments

@moonfruit
Copy link
Contributor

Please note we will close your issue without comment if you delete, do not read or do not fill out the issue checklist below and provide ALL the requested information. If you repeatedly fail to use the issue template, we will block you from ever submitting issues to Homebrew again.

  • are reporting a bug others will be able to reproduce and not asking a question. If you're not sure or want to ask a question do so on our Discourse: https://discourse.brew.sh
  • have a problem with brew install (or upgrade, reinstall) a single, official formula (not cask)? If it's a general brew problem please file this issue at Homebrew/brew: https://github.com/Homebrew/brew/issues/new/choose. If it's a brew cask problem please file this issue at https://github.com/Homebrew/homebrew-cask/issues/new/choose. If it's a tap (e.g. Homebrew/homebrew-php) problem please file this issue at the tap.
  • ran brew update and can still reproduce the problem?
  • ran brew doctor, fixed all issues and can still reproduce the problem?
  • ran brew gist-logs <formula> (where <formula> is the name of the formula that failed) and included the output link?
  • if brew gist-logs didn't work: ran brew config and brew doctor and included their output with your issue?

To help us debug your issue please explain:

  • What you were trying to do (and why)

Use option --with-gettext to enable git i18n support.

  • What happened (include command output)

Option --with-gettext has been deleted by f710a13

  • What you expected to happen

Option --with-gettext worked like old times.

  • Step-by-step reproduction instructions (by running brew install commands)
brew install git --with-gettext
@fxcoudert
Copy link
Member

See #31799 and #31510 for background

@moonfruit
Copy link
Contributor Author

moonfruit commented Sep 11, 2018

An i18n version of git is very necessary, you should not delete the option --with-gettext.

Not every options should be deleted. Create a tap to add such a simple option --with-gettext and maintain the tap is so stupid.

@SMillerDev
Copy link
Member

Out of ~81.000 installs in the last month, 16 were passing --with-gettext. I don't think it's reasonable to ask volunteers to maintain that low a usage, just because you think maintaining it yourself is "so stupid"

@DomT4
Copy link
Member

DomT4 commented Sep 11, 2018

I won't comment on the option removal here but git is a quick build & hosting a version of the formula in a tap yourself is extremely easy; that formula has taken me less than 15-20 minutes so far to create & maintain, for something that I use dozens of time a day at the bare minimum.

@chdiza
Copy link
Contributor

chdiza commented Sep 11, 2018

I don't think it's reasonable to ask volunteers to maintain that low a usage

There's no maintaining. It just sits there, untested by CI or put into bottles. It's not hurting anything.

@SMillerDev
Copy link
Member

SMillerDev commented Sep 11, 2018

Closing this as comments on the decision to remove options are better directed at the original PR or Issue.

@moonfruit
Copy link
Contributor Author

moonfruit commented Sep 12, 2018

You guys can not just remove --with-gettext because of that only some people needs it.

That I know git's i18n is pretty well. And English is not my native language, git's i18n help me a lot.

I think why that rare people install git with --gettext, because they don't know there is such a thing.

And I know that other prebuild git build with i18n by default, like git's official website and wandisco git.

I still insist do not remove --with-gettext.

If you don't like this option, why not build with i18n as default like git official website, and add a option --without-gettext, then you can see that how many people build git with --without-gettext.

@SMillerDev @DomT4 @chdiza

@SMillerDev
Copy link
Member

You could make a pull request to add it as a parameter that's always used.

@dreambottle
Copy link

I assume, this change is the reason why I started noticing this
https://public-inbox.org/git/CAKd-JgRgrJXNVyxsD-cg-TJsXu0K35tv5cSDjbsYNXCiu1j0hg@mail.gmail.com/T/

@fxcoudert
Copy link
Member

If the issue is getting reported and fixed upstream, this is good news!

@sunshineco
Copy link

The issue is not being fixed upstream (in Git) because it's not specific to Git. The same behavior is observed in other commands installed by brew (for instance, wget). So, the problem may be specific to how gettext is built by brew or may be deeper and have to do with MacOS itself. Either way, it probably deserves more investigation.

@fxcoudert
Copy link
Member

Homebrew does not do anything specific with gettext. However, it is weird, because my languages are set up in macOS so that 1 = French, 2 = English, and I correctly get the French messages from gettext-linked apps.

@sunshineco
Copy link

Indeed, the problem is specific to English being the first language, as if it's somehow special-cased to be ignored. It could be some underlying Apple API doing the special-casing upon which gettext is built (I haven't delved into it) or perhaps something is broken in gettext itself.

@fxcoudert
Copy link
Member

Look at gettext-0.19.8.1/gettext-runtime/gnulib-lib/localename.c, it has some macOS-specific code (search for Mac OS or __APPLE__). But it looks okay to me…

@storoj
Copy link

storoj commented Oct 1, 2018

I totally agree with @moonfruit that removing --with-gettext option was a mistake. But another mistake happened and gettext support became mandatory. It affects badly if your system has one or more languages. For example, I have en_RU locale and my git output became Russian. That's not an option for me and I can't help with it in a normal way. I don't want to use LC_LANG=en_US in my bash_profile because it will affect everything in my system, I don't want to make an alias like git="LC_LANG=en_US git" either.

Another point to return --with-gettext back is the way git supports i18n. According to INSTALL (https://github.com/git/git/blob/master/INSTALL) the only way to use English is exactly NO_GETTEXT flag enabled. Otherwise, po/${lang}.po (https://github.com/git/git/tree/master/po) files will be used to determine available languages. As you see, that directory does not contain "en.po" file. And if you are running git with gettext support and en_RU locale, gettext will look for following files: share/locale/{en_RU,en,ru_RU,ru}/LC_MESSAGES/git.mo. It finally finds "ru" one and uses Russian language for output which is totally unusable. I don't have any piece of development software localized in Russian.

With all the above I insist on returning --with-gettext flag back because it is the only way (designed by git authors) to use English localisation in multi-language system.

@odnoletkov
Copy link

I agree that current situation is undesirable for many users and hard to work around. Many people do specify a secondary non-en language and get an unexpected git localisation not in accordance with their preference. I think amount of people affected greatly exceeds amount of people preferring to use localised git. Until upstream supports better configuration this should be reverted back to an option for those who actually want localisation.

@fxcoudert
Copy link
Member

  • There is a known workaround: specifying a locale with environment variables.
  • Has a bug report being filed against git, gettext, or both?

@sunshineco
Copy link

Has a bug report being filed against git, gettext, or both?

Mentioned earlier, the issue was reported to Git, but it was observed that the problem was not specific to that project; wget installed by brew also experiences it, for example.

(It's possible, I suppose, that git and wget are both mis-using gettext in some way, in which case suggesting an appropriate fix to those projects would make sense.)

@sunshineco
Copy link

It seems this problem isn't new, and certainly isn't specific to Git. A similar report was made against nano two years ago (but, apparently, never resolved).

@auduchinok
Copy link
Contributor

Has a bug report being filed against git, gettext, or both?

The sad thing is it works perfectly in git bundled in Xcode tools and other distributions, with this combination of system language and locales (the same as @storoj has). It used to work in brew as well but now I just had to uninstall it and reverted to the system one.

@lock lock bot added the outdated PR was locked due to age label Nov 22, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Nov 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

No branches or pull requests

10 participants