fixing site target by running jekyll in bundle context, see #1001 #1002

original-brownbear · 2016-02-09T12:34:28Z

This PR addresses Issue #1001 reproduced and then tested via:

docker run -it -v "${PWD}:${PWD}" --rm -w "${PWD}" yegor256/rultor /bin/bash -lc 'mvn clean site -Pjekyll'

Deployments Started Failing because:

From what I can see the problem was caused by ba0d145, which added the est gem before running bundler.
est according to https://rubygems.org/gems/est/versions/0.3.1 requires a specific Nokogiri version ( 1.6.6.4).
The current Gemfile though installs nokogiri (1.6.7.2) causing a conflict shown in #994 (comment) and breaking est initially.

This conflict was then resolved in 8eaf7e2 by installing not in the system scope, now making est load fine from the system gems. The system gem context though does not load jekyll-sass now, since bundler was not run in the sudo context as per 8eaf7e2.

Also c08600f then set the Jekyll version to 2.4.0 as per the bundle. The currently available Docker Image rev. 43b7ceed592c though shows Jekyll 3.1.1 install in the system context coming Jekyll being install in the Dockerfile currently:
https://github.com/yegor256/rultor/blob/master/src/docker/Dockerfile#L106

RUN git clone https://github.com/yegor256/rultor.git && \
  cd rultor && \
  gem install jekyll && \
  mvn clean install -Pqulice --quiet && \
  cd .. && \
  rm -rf rultor

Hence running outside the bundle context will cause the wrong Jekyll version to be used even if it were to not conflict with est.

Fixed by:

This situation can be fixed by simply running Jekyll in the context of the bundle and hence not having to deal with the system nokogiri version required by est.

This is what the second command added to the pre-site execution does. In order to make the build one off in this spot I also added the bundle install to this build step.

…1001

original-brownbear · 2016-02-09T12:45:54Z

@yegor256 Pinging you here as instructed in #998 :)

alex-palevsky · 2016-02-09T14:10:57Z

@original-brownbear Thanks, I will find someone to review this PR soon

alex-palevsky · 2016-02-09T14:15:06Z

@mkordas review this please

mkordas · 2016-02-09T20:50:12Z

@original-brownbear I'm on it

mkordas · 2016-02-09T20:50:38Z

@original-brownbear please update comment #1002 (comment) to start with name tag

mkordas · 2016-02-09T20:51:54Z

@original-brownbear can you put link to issue in the PR description?

original-brownbear · 2016-02-09T20:54:56Z

@mkordas done :)

mkordas · 2016-02-09T21:34:08Z

@original-brownbear code looks ok, but it would be great if you could update PR description with:

why deployments started failing
what exactly we are fixing here

mkordas · 2016-02-09T21:38:09Z

@original-brownbear there's one more thing, just for future. According to http://at.teamed.io/qa.html

Problem reporter and problem solver are two different persons

original-brownbear · 2016-02-09T22:31:31Z

@mkordas sorry I was not aware of this rule :) Added the requested PR description now though.

mkordas · 2016-02-10T21:15:42Z

@original-brownbear description is impressive, thanks. I just need to trust you, as I'm not an expert in gems :) Yegor will confirm, but looks good to me.

mkordas · 2016-02-10T21:15:50Z

@rultor merge

rultor · 2016-02-10T21:17:18Z

@rultor merge

@mkordas Thanks for your request. @yegor256 Please confirm this.

mkordas · 2016-02-11T22:28:55Z

@yegor256 ping, this PR may solve merging issue in Rultor

yegor256 · 2016-02-12T19:47:17Z

@rultor try to merge

yegor256 · 2016-02-12T19:47:28Z

@original-brownbear many thanks, excellent investigation and fix!

rultor · 2016-02-12T19:47:38Z

@rultor try to merge

@yegor256 OK, I'll try to merge now. You can check the progress of the merge here

rultor · 2016-02-12T19:58:01Z

@rultor try to merge

@original-brownbear @yegor256 Oops, I failed. You can see the full log here (spent 10min)


Results :

Tests run: 53, Failures: 0, Errors: 0, Skipped: 3

[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 02:23 min
[INFO] Finished at: 2016-02-12T19:57:33+00:00
[INFO] Final Memory: 70M/438M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.18.1:test (default-test) on project rultor: Execution default-test of goal org.apache.maven.plugins:maven-surefire-plugin:2.18.1:test failed: The forked VM terminated without properly saying goodbye. VM crash or System.exit called?
[ERROR] Command was /bin/sh -c cd /home/r/repo && /usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java -Duser.language=en -Duser.region=US -Djava.awt.headless=true -jar /home/r/repo/target/surefire/surefirebooter1180749131843344024.jar /home/r/repo/target/surefire/surefire7222497619927761377tmp /home/r/repo/target/surefire/surefire_04246989202746292270tmp
[ERROR] -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.18.1:test (default-test) on project rultor: Execution default-test of goal org.apache.maven.plugins:maven-surefire-plugin:2.18.1:test failed: The forked VM terminated without properly saying goodbye. VM crash or System.exit called?
Command was /bin/sh -c cd /home/r/repo && /usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java -Duser.language=en -Duser.region=US -Djava.awt.headless=true -jar /home/r/repo/target/surefire/surefirebooter1180749131843344024.jar /home/r/repo/target/surefire/surefire7222497619927761377tmp /home/r/repo/target/surefire/surefire_04246989202746292270tmp
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:224)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:153)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:145)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:108)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:76)
    at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build(SingleThreadedBuilder.java:51)
    at org.apache.maven.lifecycle.internal.LifecycleStarter.execute(LifecycleStarter.java:116)
    at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:361)
    at org.apache.maven.DefaultMaven.execute(DefaultMaven.java:155)
    at org.apache.maven.cli.MavenCli.execute(MavenCli.java:584)
    at org.apache.maven.cli.MavenCli.doMain(MavenCli.java:213)
    at org.apache.maven.cli.MavenCli.main(MavenCli.java:157)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:606)
    at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced(Launcher.java:289)
    at org.codehaus.plexus.classworlds.launcher.Launcher.launch(Launcher.java:229)
    at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode(Launcher.java:415)
    at org.codehaus.plexus.classworlds.launcher.Launcher.main(Launcher.java:356)
Caused by: org.apache.maven.plugin.PluginExecutionException: Execution default-test of goal org.apache.maven.plugins:maven-surefire-plugin:2.18.1:test failed: The forked VM terminated without properly saying goodbye. VM crash or System.exit called?
Command was /bin/sh -c cd /home/r/repo && /usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java -Duser.language=en -Duser.region=US -Djava.awt.headless=true -jar /home/r/repo/target/surefire/surefirebooter1180749131843344024.jar /home/r/repo/target/surefire/surefire7222497619927761377tmp /home/r/repo/target/surefire/surefire_04246989202746292270tmp
    at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:144)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:208)
    ... 19 more
Caused by: java.lang.RuntimeException: The forked VM terminated without properly saying goodbye. VM crash or System.exit called?
Command was /bin/sh -c cd /home/r/repo && /usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java -Duser.language=en -Duser.region=US -Djava.awt.headless=true -jar /home/r/repo/target/surefire/surefirebooter1180749131843344024.jar /home/r/repo/target/surefire/surefire7222497619927761377tmp /home/r/repo/target/surefire/surefire_04246989202746292270tmp
    at org.apache.maven.plugin.surefire.booterclient.ForkStarter.fork(ForkStarter.java:515)
    at org.apache.maven.plugin.surefire.booterclient.ForkStarter.fork(ForkStarter.java:380)
    at org.apache.maven.plugin.surefire.booterclient.ForkStarter.run(ForkStarter.java:167)
    at org.apache.maven.plugin.surefire.AbstractSurefireMojo.executeProvider(AbstractSurefireMojo.java:990)
    at org.apache.maven.plugin.surefire.AbstractSurefireMojo.executeAfterPreconditionsChecked(AbstractSurefireMojo.java:824)
    at org.apache.maven.plugin.surefire.AbstractSurefireMojo.execute(AbstractSurefireMojo.java:722)
    at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:133)
    ... 20 more
[ERROR] 
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/PluginExecutionException
container 96c707d9ff83ab35c0fc222861e0257369fdeb9be8736c345496181ea9d593ac is dead
Fri Feb 12 19:57:57 UTC 2016

yegor256 · 2016-02-12T19:58:54Z

@rultor try to merge again

rultor · 2016-02-12T20:01:54Z

@rultor try to merge again

@yegor256 OK, I'll try to merge now. You can check the progress of the merge here

rultor · 2016-02-12T20:19:01Z

@rultor try to merge again

@yegor256 Done! FYI, the full log is here (took me 17min)

mkordas · 2016-02-12T20:20:19Z

@original-brownbear nice, thank you!

mkordas · 2016-02-12T20:20:23Z

@rultor deploy

rultor · 2016-02-12T20:22:17Z

@rultor deploy

@mkordas Thanks for your request. @yegor256 Please confirm this.

yegor256 · 2016-02-12T20:32:00Z

@rultor deploy

rultor · 2016-02-12T20:32:40Z

@rultor deploy

@yegor256 OK, I'll try to deploy now. You can check the progress here

rultor · 2016-02-12T20:43:15Z

@rultor deploy

@yegor256 Done! FYI, the full log is here (took me 10min)

mkordas · 2016-02-12T20:50:32Z

@original-brownbear @yegor256 any idea why this PR was not moved to merged state?

From logs:

+ sudo chown -R rultor repo
+ cd repo
container ac01fecd9a0e733950e26f3e3328ff6f15d2bc954017e58e8958992d0de0c5be is dead

original-brownbear · 2016-02-12T21:12:24Z

@mkordas I think Github is not able to recognise the push by Rultor as having merged the branch, because Rultor rebased before merging.
If you git log yegor256/master..me/1001 and the other way around you see the commit for this both times as a difference. In the git sense it's not the same commit. I think this is what is discussed in #918 and needs additional functionality in Rultor.
Put differently I think this will always happen when a --ff merge would not be possible, simply because the commit hashes change in this case and Github doesn't compare content only hashes afaik.

mkordas · 2016-02-12T21:32:57Z

@original-brownbear now I see, Rultor here is configured to do rebases before merge as ff: https://github.com/yegor256/rultor/blame/master/.rultor.yml#L26

mkordas · 2016-02-12T21:35:14Z

@yegor256 do we want to keep rebase: true for this repository?

Advantage: linear history.
Disadvantage: PRs do not get closed after merge if they are not rebased by author.

yegor256 · 2016-02-12T21:42:16Z

@mkordas that rebase:true was added there mostly for fun... we can remove it, if it's some important. are you sure the PR is not closed because of rebase?

mkordas · 2016-02-12T21:54:13Z

@yegor256 yes, I'm sure. GitHub closes pull requests only when commits from PR are transferred to master branch in unchanged way. This could be done either by merge commit or by fast-forward merge. If rebase is done, then master receives commit with different hash (committer and committer date are updated) and PR is not closed.
I see two options:

disable that feature
add possibility for rultor to close PR just after merge - but then PR will have status of closed instead of merged which also might be confusing

yegor256 · 2016-02-12T22:31:12Z

@mkordas I like the second option. rultor should do this, if rebase is set to true and should post a message to the pull request, explaining what that was done. that won't be confusing. make sense?

mkordas · 2016-02-12T22:55:56Z

@yegor256 sure, makes sense, especially with message. Should I raise ticket for that?

mkordas · 2016-02-13T15:20:26Z

@original-brownbear can you close this PR?

original-brownbear · 2016-02-13T17:23:26Z

@mkordas jup all done here : )

yegor256 · 2016-02-16T05:01:50Z

@mkordas yes, go ahead pls

original-brownbear · 2016-02-22T14:21:27Z

@mkordas I think you never created the ticket about the rebase/merge/close situation here right ?
There is no need to do this, Just going through all open tickets I found this #918 which is exactly this issue. I will add the needed information about the rebase part of things there => no need to create anything :)

mkordas · 2016-02-22T14:27:08Z

@original-brownbear you are right, thanks!

alex-palevsky · 2016-02-25T17:53:15Z

@mkordas 10 mins added to @original-brownbear account (our architect), payment ID is AP-02P21392DJ519450U, since quality is good here

fixing site target by running jekyll in bundle context, see yegor256#…

e5523f5

…1001

original-brownbear mentioned this pull request Feb 9, 2016

README should have what problems Rultor solves #998

Closed

alex-palevsky added CR labels Feb 9, 2016

mkordas mentioned this pull request Feb 10, 2016

fixed Rultor login prefix not recognized correctly in comments, when … #1000

Closed

yegor256 mentioned this pull request Feb 12, 2016

Site Target Does not Execute Jekyll in Bundle Context Causes Jekyll-Sass to not Properly Load #1001

Closed

original-brownbear closed this Feb 13, 2016

original-brownbear mentioned this pull request Feb 18, 2016

#934 Refactored EndsDaemon to use ShellCommand #1004

Closed

fixing site target by running jekyll in bundle context, see #1001 #1002

fixing site target by running jekyll in bundle context, see #1001 #1002

Conversation

original-brownbear commented Feb 9, 2016

Deployments Started Failing because:

Fixed by:

original-brownbear commented Feb 9, 2016

alex-palevsky commented Feb 9, 2016

alex-palevsky commented Feb 9, 2016

mkordas commented Feb 9, 2016

mkordas commented Feb 9, 2016

mkordas commented Feb 9, 2016

original-brownbear commented Feb 9, 2016

mkordas commented Feb 9, 2016

mkordas commented Feb 9, 2016

original-brownbear commented Feb 9, 2016

mkordas commented Feb 10, 2016

mkordas commented Feb 10, 2016

rultor commented Feb 10, 2016

mkordas commented Feb 11, 2016

yegor256 commented Feb 12, 2016

yegor256 commented Feb 12, 2016

rultor commented Feb 12, 2016

rultor commented Feb 12, 2016

yegor256 commented Feb 12, 2016

rultor commented Feb 12, 2016

rultor commented Feb 12, 2016

mkordas commented Feb 12, 2016

mkordas commented Feb 12, 2016

rultor commented Feb 12, 2016

yegor256 commented Feb 12, 2016

rultor commented Feb 12, 2016

rultor commented Feb 12, 2016

mkordas commented Feb 12, 2016

original-brownbear commented Feb 12, 2016

mkordas commented Feb 12, 2016

mkordas commented Feb 12, 2016

yegor256 commented Feb 12, 2016

mkordas commented Feb 12, 2016

yegor256 commented Feb 12, 2016

mkordas commented Feb 12, 2016

mkordas commented Feb 13, 2016

original-brownbear commented Feb 13, 2016

yegor256 commented Feb 16, 2016

original-brownbear commented Feb 22, 2016

mkordas commented Feb 22, 2016

alex-palevsky commented Feb 25, 2016