Defect #5324

Git not working if color.ui is enabled

Added by Rodrigo Toledo over 7 years ago. Updated almost 7 years ago.

Status:ClosedStart date:2010-04-15
Priority:NormalDue date:
Assignee:Jean-Baptiste Barth% Done:

100%

Category:SCM
Target version:1.0.4
Resolution:Fixed Affected version:

Description

I'm using redmine trunk and git 1.6.0.4.
I started a new project, and it didn't work with a working bare repo. Then I found that webrick was outputting this error on its log:

fatal: Not a valid object name master:

with the word "*master*" highlighted in green.

So I went to the bare repo, and running "*git branch*" I saw that the green color was on the branch list too. So I disabled the ui.color locally in that bare repo using:

git config color.ui false

and now redmine sees my repo correctly.

So, I managed to fix it on the git side, but it should be good if redmine could be able to see this by itself, and fix it or warn the user.

git--no-color.patch Magnifier (1.8 KB) Bernhard Furtmueller, 2010-04-17 23:23

failing_test_5324.txt Magnifier (5.08 KB) Jean-Baptiste Barth, 2010-11-07 16:42


Related issues

Related to Redmine - Defect #26645: git 2.14 compatibility Closed

Associated revisions

Revision 4310
Added by Jean-Baptiste Barth about 7 years ago

Force color to be disabled on git shell-outs. #5324

Contributed by Felix Schäfer

Revision 4386
Added by Jean-Baptiste Barth about 7 years ago

Added missing --no-color option in some git shell-outs. #5324

History

#1 Updated by Jean-Philippe Lang over 7 years ago

How Redmine could fix it?

#2 Updated by Jean-Philippe Lang over 7 years ago

  • Category changed from Administration to SCM

#3 Updated by Rodrigo Toledo over 7 years ago

My guess is that the issue has to do with some kind of special characters used to put color in the terminal, and that redmine is reading from that output in order to do something with the branch name. If this guess is right, the solution should be stripping that output of any special character before using it for something.

Anyway it's just a guess, I don't know anything about the internal workings of redmine.

#4 Updated by Rodrigo Toledo over 7 years ago

Maybe something like this http://www.commandlinefu.com/commands/view/3584/remove-color-codes-special-characters-with-sed but in the context of capturing any git output

#5 Updated by Bernhard Furtmueller over 7 years ago

I had a brief look to:

lib/redmine/scm/adapters/git_adapter.rb

It looks these commands could need additionally the "--no-color" option:
git branch --no-color
git log --no-color
git diff --no-color
git show --no-color
git blame --no-color <- I´m not sure if it´s really neccessary here, but it doesn´t hurt though.

#6 Updated by Bernhard Furtmueller over 7 years ago

Rodrigo, could you test the attached patch?

#7 Updated by Rodrigo Toledo over 7 years ago

That should probably work. I tried to write a test to prove it, but I don't know how should I get the git_repository that's used in the unit tests without making it from scratch.

#8 Updated by Rodrigo Toledo over 7 years ago

Somehow I cannot reproduce the issue at home, so I cannot confirm that the patch works. On monday I'll test it at work where it originally happened and I'll give confirmation.

#9 Updated by Jean-Philippe Lang over 7 years ago

I can not reproduce either. I have set:

git config --global color.ui auto

and everything still works fine (git 1.5.4.3).

#10 Updated by Bernhard Furtmueller over 7 years ago

given that it´s not for every one reproducible, I´d consider git isn´t doing the color.ui=auto thing correctly depending on gits version.

For me "git show" is colored, but when piping it to less it´s uncolored.
(git version 1.6.3.3)

#11 Updated by Felix Schäfer over 7 years ago

Can anyone confirm this is still an issue?

#12 Updated by Gustavo Delfino over 7 years ago

I am experiencing this exact same problem. git 1.6.2.3 & redmine 0.9.4

#13 Updated by Rodrigo Toledo over 7 years ago

It shouldn't hurt redmine to use --no-color on each command. Just in case.

#14 Updated by Rodrigo Toledo about 7 years ago

I found that in the machine that I had this problem, color.ui was set to "always", maybe this helps anyone trying to reproduce this issue.

#15 Updated by Felix Schäfer about 7 years ago

  • Target version set to 1.0.3

Eric, the fix for that is here: http://github.com/thegcat/redmine/commit/f9583578af6760377b4d46a5b5907f92c8d47e29

I don't think it's possible to selectively activate color other than shelling out to make the change, the change no activates color to always, so if anything ever changes in the color stuff in future git versions, we should notice through the tests. If you don't want that in, just ignore the archive of the git repo.

#16 Updated by Jean-Baptiste Barth about 7 years ago

  • Status changed from New to Resolved
  • Assignee set to Jean-Baptiste Barth
  • % Done changed from 0 to 100
  • Resolution set to Fixed

I was able to reproduce too with a recent git version (1.7.0.4) on current trunk, with color.ui set to always and no other color.* behavior defined (see git config -l|grep color ; it seems git options precedence doesn't always work as you guess).

Applied Felix's patch in r4310.

#17 Updated by Eric Davis about 7 years ago

  • Status changed from Resolved to Closed

Merged into 1.0-stable for release in 1.0.3

#18 Updated by Jean-Baptiste Barth about 7 years ago

  • Status changed from Closed to Reopened
  • Target version changed from 1.0.3 to 1.0.4

--no-color still missing in some places. http://ci.finn.de/ is all green, but I suspect it's because tmp/test/git_repository is still the old one. Fixed in r4386.

I tested it with GIT 1.5.6.5 and 1.7.1, if anyone finds a problem with a different version let us know before it's merged in stable branch.

#19 Updated by Jean-Baptiste Barth about 7 years ago

  • Status changed from Reopened to Resolved

#20 Updated by Felix Schäfer about 7 years ago

Jean-Baptiste Barth wrote:

--no-color still missing in some places.

Not all git commands support --no-color (or at least have an explicit colored output…). Did you encounter any test errors? They did all pass when I tested it here.

#21 Updated by Jean-Baptiste Barth about 7 years ago

git show supports it at least from 1.5.6.5.

I don't remember having found a failing test when committed it, but I found this one yesterday when investigating something else:

% RAILS_ENV=test ruby test/functional/repositories_git_controller_test.rb -n test_diff > failing_test_5324.txt 2>&1

See result attached. No output from app/views/common/_diff partial, and when I put the debugger in the view, I saw the diff output was full of color codes.

#22 Updated by Felix Schäfer about 7 years ago

Odd. Anyway, the sure-fire way would be to add it to GIT_BIN, but I didn't want to add it there lest it breaks something else…

#23 Updated by Jean-Philippe Lang almost 7 years ago

  • Status changed from Resolved to Closed

r4386 merged in 1.0-stable for 1.0.4 release.

#24 Updated by Toshi MARUYAMA 4 months ago

Also available in: Atom PDF