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

>=1.4.0 freezes in life-lock when trying to edit configuration #101

Closed
1 task done
jgsuess opened this issue Nov 7, 2018 · 34 comments
Closed
1 task done

>=1.4.0 freezes in life-lock when trying to edit configuration #101

jgsuess opened this issue Nov 7, 2018 · 34 comments
Labels

Comments

@jgsuess
Copy link

jgsuess commented Nov 7, 2018

version: >=1.4.0

usage context:

  • maven command line: 3.5.4

Problem description:

  • After upgrade from jgitver 1.3.x to 1.4.0 and following versions, our build gets stuck in a live-lock
    (Java seems to be polling). Below is a maven debug log of the situation.

Checking the commands I find that the output for the version command is retrieved correctly, but that
git config --system --edit starts my nano editor. I am not sure if this is intended.

2953 [INFO] using jgitver configuration file: /home/josuess/git/util/.mvn/jgitver.config.xml
2962 [DEBUG] ignoring directory (& sub dirs): /home/josuess/git/util/.m2
3112 [DEBUG] readpipe [git, --version],/usr/bin
3131 [DEBUG] readpipe may return 'git version 2.19.1'
3131 [DEBUG] remaining output:

3132 [DEBUG] readpipe [git, config, --system, --edit],/usr/bin
3136 [DEBUG] readpipe may return '/etc/gitconfig'
3136 [DEBUG] remaining output:
@McFoggy
Copy link
Contributor

McFoggy commented Nov 7, 2018

@jgsuess the debug log with git commands should not come from jgitver (using jgit) but probably from your maven-scm configuration.

Do you have a simplified reproducer case?

@jgsuess
Copy link
Author

jgsuess commented Nov 7, 2018

No, not at this time. However, our while we have scm defined, none of our plugins do SCM operations. Checkouts are executed by the CI system. Could this by JGIt? Has there been a version change in JGitver that uses a new JGit.

@McFoggy
Copy link
Contributor

McFoggy commented Nov 7, 2018

but AFAIK jgit is a full java implementation of git

Thus commands like 3112 [DEBUG] readpipe [git, --version],/usr/bin cannot come from it.

How did you took the log? is it only the result of mvn -X or is it the debug output of your CI?

@McFoggy
Copy link
Contributor

McFoggy commented Nov 7, 2018

there is for sure one of the plugins that has a dependency on some maven-scm projects.
Can you remove the SCM section to see what plugin is failing?

don't you extract git metadatas using another plugin like build-number or something like that?

@jgsuess
Copy link
Author

jgsuess commented Nov 7, 2018 via email

@McFoggy
Copy link
Contributor

McFoggy commented Nov 8, 2018

OK I reproduced the log, but not the outage

docker run --rm -it -v %CD%:/sources -w /sources maven:3.3.9-jdk-8 /bin/bash

mvn archetype:generate \
  -DgroupId=fr.brouillard.oss.jgitver \
  -DartifactId=mvn-sample \
  -DarchetypeArtifactId=maven-archetype-quickstart \
  -DinteractiveMode=false \
  -DarchetypeCatalog=internal

cd mvn-sample
echo target/ > .gitignore
echo *.log >> .gitignore
echo # jgitver mvn-sample > README.md
git init
git config user.name "Bob TheBuilder"
git config user.email "[email protected]"
git add .
git commit -m "add project to SCM"
sh -c "$(wget https://raw.githubusercontent.com/jgitver/jgitver-maven-plugin/master/src/doc/scripts/install.sh -O -)"
git add .
git commit -m "add jgitver"
mvn validate
mvn -X validate | tee build.log
grep -n readpipe build.log

I'll investigate now.

@McFoggy
Copy link
Contributor

McFoggy commented Nov 8, 2018

indeed it is coming from jgit

image

but as far as I can understand the jgit sources it is an intended behavior and it is not supposed to be blocking because the EDITOR is changed before calling.

https://git.eclipse.org/r/plugins/gitiles/jgit/jgit/+/v4.9.1.201712030800-r/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java#649

but here, even if it is the source for the jgit version jgitver declares to use, we are not on the same line than in my maven 3.3.9 debug session ; so jgit version should not be the one expected by jgitver.

the sources are the good ones, I stopped on the part that lookup the git exe.
the code should not block...

@McFoggy
Copy link
Contributor

McFoggy commented Nov 8, 2018

@jgsuess did you succeed in reproducing the issue? I reproduce the log [DEBUG] readpipe [git, config, --system, --edit],/usr/bin but I cannot have the full process to block.

Don't you have a modified version of the echo command on your system?

@McFoggy
Copy link
Contributor

McFoggy commented Nov 8, 2018

also

  • can you provide the description of the full system : java version, git version, OS, ...
  • on your CI system, on the blocking project, does the following command GIT_EDITOR=echo git config --system --edit correctly ends? and how many lines are returned?

@McFoggy
Copy link
Contributor

McFoggy commented Nov 8, 2018

finally there were no changes in jgit version between 1.3.0 and 1.4.x.

the change appeared between 1.2.1 & 1.3.0 ; but jgit actual version was already there in 1.3.0

@jgsuess
Copy link
Author

jgsuess commented Nov 8, 2018 via email

@McFoggy
Copy link
Contributor

McFoggy commented Nov 9, 2018

@jgsuess I have searched and put all what you ask above in the discussion.

I assume that jgit wants to do some stream editing, vi style,
but it does not see the response it expects.

exactly, you can look at jgit code here : https://git.eclipse.org/r/plugins/gitiles/jgit/jgit/+/v4.9.1.201712030800-r/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java#649

please provide more information on your setup on the CI box that is failing:

  • OS + OS version
  • maven version
  • java version
  • git version
  • attach "git-config.out" file, result of : GIT_EDITOR=echo git config --system --edit > git-config.out

I am willing to help you but I need more info ; I still have no reproducer where the call is blocking.

@McFoggy
Copy link
Contributor

McFoggy commented Nov 14, 2018

@jgsuess any news?

without further help/info on your system setup I don't know what to do more for you.

@genuss
Copy link

genuss commented Dec 6, 2018

I have the same bug on my windows desktop. On a simple project mvn clean freezes on the same line. The last lines from lag are:

[DEBUG] readpipe [git, config, --system, --edit],C:\Users\genus\Desktop\work\soft\cmder\vendor\git-for-windows\bin
[DEBUG] readpipe may return 'C:/Users/genus/Desktop/work/soft/cmder/vendor/git-for-windows/mingw64/etc/gitconfig'
[DEBUG] remaining output:

Versions info

Windows Version 10.0.17134.112
Apache Maven 3.5.3 (3383c37e1f9e9b3bc3df5050c29c8aff9f295297; 2018-02-24T22:49:05+03:00)
Maven home: C:\java\apache-maven-3.5\bin\..
Java version: 1.8.0_181, vendor: Azul Systems, Inc.
Java home: C:\java\zulu8.31.0.1-jdk8.0.181-win_x64\jre
Default locale: ru_RU, platform encoding: Cp1251
OS name: "windows 10", version: "10.0", arch: "amd64", family: "windows"
git version 2.16.1.windows.4

In powershell:

$Env:GIT_EDITOR = 'echo';
git config --system --edit;

Outputs hint: Waiting for your editor to close the file... C:/Users/genus/Desktop/work/soft/cmder/vendor/git-for-windows/mingw64/etc/gitconfig

In cmd

set GIT_EDITOR=echo;
git config --system --edit

Outputs the samehint: Waiting for your editor to close the file... C:/Users/genus/Desktop/work/soft/cmder/vendor/git-for-windows/mingw64/etc/gitconfig

I suppose there is something wrong as jgit works with environment variables.

@McFoggy
Copy link
Contributor

McFoggy commented Dec 6, 2018

@genuss @jgsuess honestly I do not know how to go on.

I also have the same warning on windows

14:51:04 D:\dev\tmp\mvn-sample>git config --system --edit
hint: Waiting for your editor to close the file... D:/dev/tools/SCMs/git/Git/mingw64/etc/gitconfig

but it does not block anything ; it just works for me

14:50:52 D:\dev\tmp\mvn-sample>ver

Microsoft Windows [version 10.0.16299.547]

14:50:56 D:\dev\tmp\mvn-sample>git --version
git version 2.16.2.windows.1

14:51:00 D:\dev\tmp\mvn-sample>mvn --version
Apache Maven 3.5.3 (3383c37e1f9e9b3bc3df5050c29c8aff9f295297; 2018-02-24T20:49:05+01:00)
Maven home: D:\dev\tools\apache\maven\current\bin\..
Java version: 1.8.0_162, vendor: Oracle Corporation
Java home: D:\dev\tools\jdks\current\jre
Default locale: fr_FR, platform encoding: Cp1252
OS name: "windows 10", version: "10.0", arch: "amd64", family: "windows"

14:51:04 D:\dev\tmp\mvn-sample>git config --system --edit
hint: Waiting for your editor to close the file... D:/dev/tools/SCMs/git/Git/mingw64/etc/gitconfig

14:51:15 D:\dev\tmp\mvn-sample>mvn validate
[INFO] no suitable configuration file found, using defaults
[INFO] Scanning for projects...
[INFO] Using jgitver-maven-plugin [1.4.3] (sha1: 0d071989527a5b3dd75f70687603c1147a58a130)
[INFO] jgitver-maven-plugin is about to change project(s) version(s)
[INFO]     fr.brouillard.oss.jgitver::mvn-sample::1.0-SNAPSHOT -> 0.0.0-SNAPSHOT
[INFO]
[INFO] ----------------< fr.brouillard.oss.jgitver:mvn-sample >----------------
[INFO] Building mvn-sample 0.0.0-SNAPSHOT
[INFO] --------------------------------[ jar ]---------------------------------
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 1.026 s
[INFO] Finished at: 2018-12-06T14:51:27+01:00
[INFO] ------------------------------------------------------------------------

14:51:27 D:\dev\tmp\mvn-sample>

@genuss
Copy link

genuss commented Dec 6, 2018

Just made some debugging on jgitver 0.8.6 and it has something to do with jgitver/jgitver#63
You added maxDepth parameter but it's always set to Integer.MAX_INT. I tried to set it to 50 and it worked. Looks like the fr.brouillard.oss.jgitver.GitVersionCalculator#lookupCommits is a cause of all troubles.

So this issue has nothing to do with git or jgit.

@McFoggy
Copy link
Contributor

McFoggy commented Dec 6, 2018

ok thus it does not occur on small repos but on your big ones ; right?
do you have one that is public? that I can test on?

@McFoggy
Copy link
Contributor

McFoggy commented Dec 6, 2018

in fact the issue comes from jgitver/jgitver#49

jgitver now tries to better handle merged branches.
On big projects, for which a lot of branches are merged without rebase on master (like gradle as exposed in jgitver/jgitver#63) there exists A LOT of existing parent path in the git graph that jgitver looks-up t find a suitable version.
the maxDepth property is a quick and dirty answer not to go too deep in the hierarchy.

@genuss
Copy link

genuss commented Dec 6, 2018

Exactly, I was just typing that our repos aren't very large but have many branches which are merged one into another witout rebases.
Unfortunately they are fully closed source, so I cannot share them. But I looked around and found https://github.com/apache/spark.git where I could reproduce the issue.

@jgsuess
Copy link
Author

jgsuess commented Dec 7, 2018

Sorry @McFoggy for leading you on a spin... Would it be possible to have a watchdog that checks on the fan-out? Than you would get a warning after time, rather than setting bounds in advance?

@McFoggy
Copy link
Contributor

McFoggy commented Dec 7, 2018

I have several things in mind to resolve that.

1. allow to set a maxDepth from the maven plugin

As I already did it for the gradle one, I am going to deliver a version of the maven plugin that allows you to define a maxDepth to look for tags. This is for me just a transient solution to workaround the issue.

2. introduce a real search policy

This would allow, depending on: the projects, the branching policy to be able for example not to lookup merged branches, to define max depth search, ....

3. add a resolution cache

For big repositories, that have lot of merges with no rebase policy, tags on merged branches (release branches) we could end in having to navigate thru all parents ; like it is today and is failing !
My idea would be to introduce a task which role would be to pre-compute/pre-parse the history graph and keep enough information so that the next parsing will be able to reuse partial information found in the cache.
But this would probably mean that the cache is put under source control and updated from time to time.
There could be 2 caches in fact, one in SCM (updated from time) and one computed above the stored one ; so that if the stored cache is old, then you local one can be kept up to date and thus still deliver good performances.

I will do (1) probably today so that you can at least bypass the issue by defining a max depth search.

@djarosz @jgsuess @genuss please raise you hand or comment on those ; I am of course interested in your feedback.

@genuss
Copy link

genuss commented Dec 7, 2018

I have a question. I don't quite understand the need of traversing git tree, why do we actually need this? Is it not just sufficient to get the branches the current head is on?

And a little comment: third option should be our the last resort:)

@McFoggy
Copy link
Contributor

McFoggy commented Dec 7, 2018

get the branches the current head is on

I think you meant the commits of the branch your are on.

I am really open to hear for ideas on how to go ahead.

@McFoggy
Copy link
Contributor

McFoggy commented Dec 7, 2018

also we have to make some awareness that using jgitver there should be tags somehow in order to make it work.

for spark for example

spark>git log --oneline | wc -l
  23345

spark>git log --oneline | grep "tag:"

spark>

on the master branch there is not tag at all.

In the same way we are not compatible with projects that do a short lived branch (non merged into master) to do the release ; like you can find for example if you migrate a project using maven-release with SVN into git.

@McFoggy McFoggy closed this as completed in f7ed5bc Dec 7, 2018
@McFoggy
Copy link
Contributor

McFoggy commented Dec 7, 2018

for the moment, please update to jgitver-maven-plugin-1.4.4 and set the new configuration property maxSearchDepth

<configuration>
    <maxSearchDepth>100</maxSearchDepth>      put whatever best fits your project
</configuration>

@genuss
Copy link

genuss commented Dec 10, 2018

I see now what was the core issue in my projects. The tag names were not just 1.2.3 but some-letters-1.2.3 and jgitver couldn't identify versions from them, so it tried to find them and like spark repo couldn't do it at all.
maxSearchDepth works like a charm now! Thanks a lot! I think we can leave that as is for now. Because snapshot versions are not a big deal anyway.

@McFoggy
Copy link
Contributor

McFoggy commented Dec 10, 2018

@genuss you can also configure the regexp for the tags if you do not use a standard pattern

@McFoggy McFoggy mentioned this issue Dec 27, 2018
@McFoggy
Copy link
Contributor

McFoggy commented Jan 7, 2019

as stated in this comment I just released jgitver-0.9.0 & jgitver-maven-plugin-1.4.5.
Both versions should have drastically improved performances.

Can you confirm please?

PS: you should be able to remove maxSearchDepth parameter from your configurations.

@genuss
Copy link

genuss commented Jan 9, 2019

@McFoggy looks great! Just tested with one of my projects. The initial configuration was:

<configuration xmlns="http://jgitver.github.io/maven/configuration/1.0.0-beta"
               xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
               xsi:schemaLocation="http://jgitver.github.io/maven/configuration/1.0.0-beta https://jgitver.github.io/maven/configuration/jgitver-configuration-v1_0_0-beta.xsd ">
    <regexVersionTag>(?:[\w-]+-)?([0-9.]+(?:-RC[0-9]+)?)</regexVersionTag>
    <maxSearchDepth>10</maxSearchDepth>
</configuration>

Version computed for ~2.7 sec. When I removed maxSearchDepth it was the same, when I removed regexVersionTag, it was even better like ~1.4 sec.
Great job, thank you!

@McFoggy
Copy link
Contributor

McFoggy commented Jan 9, 2019

great !

stay tuned, I am going to release quite soon jgitver/jgitver#70 and will offer more policies depending on your project size, branching model, tagging policy, ...

@jgsuess
Copy link
Author

jgsuess commented Jan 9, 2019 via email

@DrVanScott
Copy link

Can you please add some documention for the new parameter?

What is limited by this parameter? The number of commits, the number of merges?

I want to set this limit not too high to speed up the process, on the other hand it stops to work if i set the limit below "5", as the plugin then starts to use "0.0.0" as fallback version...

Or is there a way to make maven fail if the plugin returns "0.0.0"?

@McFoggy
Copy link
Contributor

McFoggy commented Jan 11, 2019

@DrVanScott

Can you please add some documention for the new parameter?

sure I'd like to but I also have to find some time to.

My advise on jgitver-maven-plugin >= 1.4.5, remove the maxSearchDepth ; it was only introduced to workaround the issues we had in the prvious versions. You should not use it anymore.

Moreover, I'll try to release this week-end both jgitver-0.10.0 & jgitver-maven-plugin-1.5.0 which will bring jgitver/jgitver#70 live.

@DrVanScott
Copy link

@McFoggy : Thanks a lot! I can confirm that 1.4.5 is working for me (without maxSearchDepth)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants