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.

Sign up for GitHub

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

Jump to bottom

Adding gradle support #75

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

Adding gradle support #75

wants to merge 3 commits into from

Conversation

Napo2k
Copy link

@Napo2k Napo2k commented Aug 11, 2020

When working into taking MPL into my current company, I made an effort to make it compatible with gradle - since we use gradle a lot more than maven
This PR brings those changes back into mainline, hopefully without breaking maven support

@Napo2k
Copy link
Author

Napo2k commented Aug 11, 2020 •
edited
Loading

In the CircleCI interface, is there a way that I can see what the exact command is being run? I can see

#!/bin/bash -eo pipefail
docker run -e "MPL_VERSION=${CIRCLE_SHA1}" -e "MPL_CLONE_URL=$(echo "${CIRCLE_REPOSITORY_URL}" | sed 's|git@github.com:|https://github.com/|')" --name jenkins --rm -it --volumes-from jenkins-home jenkinsbro-master

but I'd like to see the value of the variables :)

I'd also love to see how exactly is the test pipeline job configured...

From MPLPipelineTest.groovy

    def scm = new GitSCM(MODULE.clone_url)
    scm.branches = [new BranchSpec(MODULE.branch_spec ?: 'master')]
    scm.userRemoteConfigs = GitSCM.createRepoList(MODULE.clone_url, MODULE.credentials_id ?: '')

Is it using the right repo? :D

sparshev
sparshev requested changes Aug 11, 2020
View reviewed changes
Copy link
Collaborator

@sparshev sparshev left a comment

Choose a reason for hiding this comment

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

So overall - the new build system should follow use the same rules as the default one. For sure here should be no binaries in the change (check wrapper jar).

That's the things I found for now.

.gitignore
@@ -1 +1,8 @@
/target
.idea
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please check the list - here should be just the ignores related to the project, not your environment. Your env settings you can put to global gitignore of your environment.

.gitignore
mpl.iml
.factorypath
.gradle/**
build/**
Copy link
Collaborator

Choose a reason for hiding this comment

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

The POSIX text line ends with newline symbol, so any text file should contain the newline in the end.

build.gradle
}

dependencies {
compile group: 'org.codehaus.groovy', name: 'groovy-all', version: '3.0.3'
Copy link
Collaborator

Choose a reason for hiding this comment

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

The versions should be in sync with maven configs.

build.gradle
compile group: 'com.lesfurets', name: 'jenkins-pipeline-unit', version: '1.1'

testCompile group: 'junit', name: 'junit', version: '4.12'
testCompile group: 'org.mockito', name: 'mockito-core', version: '2.+'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why mockito is here? The deps should be quite the same as in maven build - minimal number of deps.

build.gradle
}
}

jacocoTestReport {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like some custom report not actually needed or should be in sync with maven.

build.gradle
dependsOn prepareForTests
// using grape causes issues with missing classpaths
// as gradle does not use ivy by default.
systemProperty 'groovy.grape.enable', 'false'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmmm, I thought maven allows to use grapes and they could be used by tests sometimes, maybe to figure out what's wrong with it instead of just disabling? Maybe it's just your env issue?

resources/com/griddynamics/devops/mpl/modules/Build/Build.groovy
@@ -2,7 +2,11 @@
* Common build module
*/

MPLModule('Maven Build', CFG)
if ( fileExists('build.gradle')) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

You changing the interface here, there should be other way because default build system is maven, but still way to use gradle build for circleci automation (also need to be added as a separated pipeline).

resources/com/griddynamics/devops/mpl/modules/Build/GradleBuild.groovy
* Simple Gradle Build
*/

sh './gradlew clean build'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmmm, not sure it's a great jenkins-way - maybe use tool of jenkins? Just imagine the strict environment where is no access to the internet (quite common in the most companies).

test/groovy/com/griddynamics/devops/mpl/modules/Build/BuildTest.groovy
@@ -24,6 +24,8 @@
import org.junit.Before
import org.junit.Test

import java.nio.file.Paths
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why it's needed?

test/groovy/com/griddynamics/devops/mpl/modules/Build/BuildTest.groovy
@@ -40,6 +42,8 @@ class BuildTest extends MPLTestBase {
void setUp() throws Exception {
String sharedLibs = this.class.getResource('.').getFile()

println("sharedLibs: ${sharedLibs}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why needed?

@sparshev
Copy link
Collaborator

And about hte build - yeah, looks like it not supports the fork repos for now, will check what to do to fix that.

@sparshev sparshev requested a review from pazovtsev August 11, 2020 19:46
@sparshev
Copy link
Collaborator

Hi @Napo2k , I fixed the issue with building from the fork - so please rebase your changes on top of master branch.

@Napo2k
Copy link
Author

Napo2k commented Aug 14, 2020

I will implement the changes when I have time
FWIW: I copied the build.gradle I was using locally, so it might be "polluted" to a certain degree. Some dependencies I had to fix since it wasn't able to find some classes when compiling or running tests.
Thanks a lot for your feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sparshev sparshev sparshev requested changes

@pazovtsev pazovtsev Awaiting requested review from pazovtsev

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Napo2k @sparshev