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

Avoid unnecessary copying of system lib files #5418

Merged
merged 1 commit into from
Jul 15, 2024

Conversation

annaibm
Copy link
Contributor

@annaibm annaibm commented Jul 4, 2024

  • Ensure system lib files are only copied when IS_SVT_TESTREPO is set to true
  • Prevent unnecessary copying of system lib files to systemtest_prereqs directory

related:https://github.ibm.com/runtimes/backlog/issues/1457

@annaibm
Copy link
Contributor Author

annaibm commented Jul 5, 2024

[echo] isSVTTestRepo is true
17:16:46      [mkdir] Created dir: /home/jenkins/workspace/Grinder_CR/jvmtest/system/systemtest_prereqs
17:16:46      [mkdir] Created dir: /home/jenkins/workspace/Grinder_CR/aqa-tests/systemtest_prereqs
17:16:46       [copy] Copying 11 files to /home/jenkins/workspace/Grinder_CR/jvmtest/system/systemtest_prereqs
17:16:46       [copy] Copying 11 files to /home/jenkins/workspace/Grinder_CR/aqa-tests/systemtest_prereqs
17:16:46  
common_dist:
17:25:48       [copy] Copying 330 files to /home/jenkins/workspace/Grinder_testList_0/jvmtest/system/STF
17:25:49       [copy] Copying 1311 files to /home/jenkins/workspace/Grinder_testList_0/jvmtest/system/aqa-systemtest
17:25:49       [copy] Copying 294 files to /home/jenkins/workspace/Grinder_testList_0/jvmtest/system/openj9-systemtest
17:25:50       [echo] env.IS_SVT_TESTREPO is ${env.IS_SVT_TESTREPO}
17:25:50       [echo] isSVTTestRepo is false

buildenv/jenkins/JenkinsfileBase Outdated Show resolved Hide resolved
system/common.xml Outdated Show resolved Hide resolved
@annaibm
Copy link
Contributor Author

annaibm commented Jul 9, 2024

Grinder link:
VENDOR_TEST_REPOS: [email protected]:runtimes/SVTTestRepo.git
TARGET: jssejce.JCE_2
https://hyc-runtimes-jenkins.swg-devops.com/job/Grinder_CR/19279/consoleFull

Grinder link:
TARGET: extended.system
https://hyc-runtimes-jenkins.swg-devops.com/job/Grinder/41959/

system/common.xml Outdated Show resolved Hide resolved
Copy link
Contributor

@LongyuZhang LongyuZhang left a comment

Choose a reason for hiding this comment

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

LGTM

@annaibm annaibm marked this pull request as ready for review July 10, 2024 15:34
Comment on lines 354 to 359
<condition property="IS_SVT_TESTREPO" value="true" else="false">
<and>
<isset property="env.IS_SVT_TESTREPO"/>
<equals arg1="${env.IS_SVT_TESTREPO}" arg2="true"/>
</and>
</condition>
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this condition check? Can we do it at line 364?

<equals arg1="${env.IS_SVT_TESTREPO}" arg2="true"/>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure , I will try doing there.

@llxia
Copy link
Contributor

llxia commented Jul 11, 2024

Please provide Grinder link based on the latest change.

@@ -112,7 +112,7 @@ def setupEnv() {

env.LIGHT_WEIGHT_CHECKOUT = (params.LIGHT_WEIGHT_CHECKOUT == false) ? params.LIGHT_WEIGHT_CHECKOUT : true
env.GENERATE_JOBS = params.GENERATE_JOBS ?: false
env.isSVTTestRepo=false
env.IS_SVT_TESTREPO = ""
Copy link
Contributor

Choose a reason for hiding this comment

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

I do not think this line is needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok,I was providing this line to check this variable is set or not set. Will try removing this line and test.

@@ -511,6 +511,8 @@ def setup() {
VENDOR_TEST_BRANCHES = (params.VENDOR_TEST_BRANCHES) ? "--vendor_branches \"${params.VENDOR_TEST_BRANCHES}\"" : ""
VENDOR_TEST_DIRS = (params.VENDOR_TEST_DIRS) ? "--vendor_dirs \"${params.VENDOR_TEST_DIRS}\"" : ""
VENDOR_TEST_SHAS = (params.VENDOR_TEST_SHAS) ? "--vendor_shas \"${params.VENDOR_TEST_SHAS}\"" : ""
env.IS_SVT_TESTREPO = params.VENDOR_TEST_REPOS.contains('SVTTestRepo') ? true : false
Copy link
Contributor

Choose a reason for hiding this comment

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

This may fail if params.VENDOR_TEST_REPOS is not set.

Copy link
Contributor

Choose a reason for hiding this comment

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

#5418 (comment) passed because params.VENDOR_TEST_REPOS is set to empty, not undefined. I think we should do

env.IS_SVT_TESTREPO = (params.VENDOR_TEST_REPOS  && params.VENDOR_TEST_REPOS.contains('SVTTestRepo')) ? true : false

@annaibm
Copy link
Contributor Author

annaibm commented Jul 12, 2024

Grinder links:
TARGET: MiniMix_5m, when VENDOR_TEST_REPOS is not set
https://hyc-runtimes-jenkins.swg-devops.com/job/Grinder/42038/
TARGET: extended.system, VENDOR_TEST_REPOS is set
https://hyc-runtimes-jenkins.swg-devops.com/job/Grinder/42034/

Not copying

 
09:27:08  common_dist:
09:27:08       [copy] Copying 330 files to /home/jenkins/workspace/Grinder/jvmtest/system/STF
09:27:08       [copy] Copying 1311 files to /home/jenkins/workspace/Grinder/jvmtest/system/aqa-systemtest
09:27:09       [copy] Copying 294 files to /home/jenkins/workspace/Grinder/jvmtest/system/openj9-systemtest

When VENDOR_TEST_REPOS: [email protected]:runtimes/SVTTestRepo.git
https://hyc-runtimes-jenkins.swg-devops.com/job/Grinder_CR/19670/consoleFull

build:
11:03:16  
11:03:16  common_dist:
11:03:16       [copy] Copying 336 files to C:\Users\jenkins\workspace\Grinder_CR\jvmtest\system\STF
11:03:20       [copy] Copying 1203 files to C:\Users\jenkins\workspace\Grinder_CR\jvmtest\system\aqa-systemtest
11:03:37       [copy] Copied 510 empty directories to 6 empty directories under C:\Users\jenkins\workspace\Grinder_CR\jvmtest\system\aqa-systemtest
11:03:37       [copy] Copying 298 files to C:\Users\jenkins\workspace\Grinder_CR\jvmtest\system\openj9-systemtest
11:03:39      [mkdir] Created dir: C:\Users\jenkins\workspace\Grinder_CR\jvmtest\system\systemtest_prereqs
11:03:39      [mkdir] Created dir: C:\Users\jenkins\workspace\Grinder_CR\aqa-tests\systemtest_prereqs
11:03:39       [copy] Copying 171 files to C:\Users\jenkins\workspace\Grinder_CR\jvmtest\system\systemtest_prereqs
11:03:45       [copy] Copying 171 files to C:\Users\jenkins\workspace\Grinder_CR\aqa-tests\systemtest_prereqs
11:03:49  

- Ensure system lib files are only copied when isSVTTestRepo is set to true
- Prevent unnecessary copying of system lib files to systemtest_prereqs directory

related:https://github.ibm.com/runtimes/backlog/issues/1457

Signed-off-by: Anna Babu Palathingal <[email protected]>
@annaibm
Copy link
Contributor Author

annaibm commented Jul 15, 2024

Grinder links:
TARGET: MiniMix_5m, when VENDOR_TEST_REPOS is not set
https://hyc-runtimes-jenkins.swg-devops.com/job/Grinder/42043/

When VENDOR_TEST_REPOS: [email protected]:runtimes/SVTTestRepo.git
TARGET: jssejce.JCE_2
https://hyc-runtimes-jenkins.swg-devops.com/job/Grinder_CR/19805/
TARGET: cookie
https://hyc-runtimes-jenkins.swg-devops.com/job/Grinder_CR/19804/

@llxia llxia merged commit 38db5e7 into adoptium:master Jul 15, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants