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

Add includeBuildNumberInTargetPath configuration parameter #169

Merged
merged 14 commits into from
Jan 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions README.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ using those artifacts. false for default if the parameter isn't specified
(Snippet Generator defaults this to true and specifies the parameter).

|resultVariableSuffix |boolean |useless for pipelines
|includeBuildNumberInTargetPath |boolean |Include source build number in target path.
|===
* selectors
+
Expand Down
12 changes: 12 additions & 0 deletions src/main/java/hudson/plugins/copyartifact/CopyArtifact.java
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ public class CopyArtifact extends Builder implements SimpleBuildStep {
private String project;
private String parameters;
private String filter, target;
private boolean includeBuildNumberInTargetPath;
private String excludes;
private /*almost final*/ BuildSelector selector;
@Deprecated private transient Boolean stable;
Expand Down Expand Up @@ -201,6 +202,7 @@ public CopyArtifact(String projectName) {
setOptional(false);
setFingerprintArtifacts(false);
setResultVariableSuffix(null);
setIncludeBuildNumberInTargetPath(false);
}

@DataBoundSetter
Expand Down Expand Up @@ -233,6 +235,11 @@ public void setFlatten(boolean flatten) {
this.flatten = flatten ? Boolean.TRUE : null;
}

@DataBoundSetter
public void setIncludeBuildNumberInTargetPath(final boolean includeBuildNumberInTargetPath) {
this.includeBuildNumberInTargetPath = includeBuildNumberInTargetPath;
}

@DataBoundSetter
public void setOptional(boolean optional) {
this.optional = optional ? Boolean.TRUE : null;
Expand Down Expand Up @@ -364,6 +371,10 @@ public String getResultVariableSuffix() {
return resultVariableSuffix;
}

public boolean getIncludeBuildNumberInTargetPath() {
return this.includeBuildNumberInTargetPath;
}

private boolean upgradeIfNecessary(AbstractProject<?,?> job) throws IOException {
if (isUpgradeNeeded()) {
Jenkins jenkins = Jenkins.getInstanceOrNull();
Expand Down Expand Up @@ -501,6 +512,7 @@ public void perform(@NonNull Run<?, ?> build, @NonNull FilePath workspace, @NonN
if (target.length() > 0) {
targetDir = new FilePath(targetDir, env.expand(target));
}
if (this.includeBuildNumberInTargetPath) targetDir = new FilePath(targetDir, String.valueOf(src.getNumber()));
expandedFilter = env.expand(filter);
if (expandedFilter.trim().length() == 0) {
expandedFilter = "**";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ THE SOFTWARE.
<st:nbsp/> <st:nbsp/> <st:nbsp/> <st:nbsp/> <st:nbsp/>
<f:checkbox field="fingerprintArtifacts" default="true"/>
<label class="attach-previous">${%Fingerprint Artifacts}</label>
<st:nbsp/> <st:nbsp/> <st:nbsp/> <st:nbsp/> <st:nbsp/>
<f:checkbox field="includeBuildNumberInTargetPath" default="false"/>
<label class="attach-previous">${%Include Build Number}</label>
</f:entry>
<f:advanced>
<f:entry title="${%Result variable suffix}" field="resultVariableSuffix">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,6 @@ Artifacts\ to\ copy=Zu kopierende Artefakte
Target\ directory=Zielverzeichnis
Flatten\ directories=Verzeichnisse reduzieren
Fingerprint\ Artifacts=Fingerabdr\u00fccke von Artefakten erstellen
Optional=Optional
Optional=Optional
Fingerprint\ Artifacts=
Include\ Build\ Number=
Comment on lines +8 to +9
Copy link
Member

Choose a reason for hiding this comment

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

This will translate the strings to an empty string. Almost certainly not what you want.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @daniel-beck . I saw that in the code review and wondered about it. When I searched through Jenkins core and saw a number of message properties that were assigned an empty value, I thought that it must be a pattern that caused the property to be assigned the value from the default language.

An example:

https://github.com/jenkinsci/jenkins/blob/a530a99926940cb37fd2f7b66e5c2f458254a710/core/src/main/resources/hudson/logging/LogRecorder/configure_fr.properties#L24

Another example:

https://github.com/jenkinsci/jenkins/blob/a530a99926940cb37fd2f7b66e5c2f458254a710/core/src/main/resources/hudson/model/RunParameterDefinition/config_fr.properties#L24

Is the preferred pattern to leave the untranslated key out of the localized property file or is there a better way to handle it?

In this case, the best way to handle it may be to transition this plugin to use CrowdIn for its localization. That will allow translators to submit messages through the CrowdIn user interface rather than directly editing property files.

@allancth would you like to submit the pull request to remove the untranslated properties or would you prefer that I do it so that you can approve the change?

Copy link
Contributor

Choose a reason for hiding this comment

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

@allancth I submitted:

If you approve and merge, it should result in a new release with a changelog that includes the bug fix.

Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,6 @@ Artifacts\ to\ copy=\u30B3\u30D4\u30FC\u3059\u308B\u6210\u679C\u7269
Artifacts\ not\ to\ copy=\u30b3\u30d4\u30fc\u3057\u306a\u3044\u6210\u679c\u7269
Target\ directory=\u30B3\u30D4\u30FC\u5148\u30C7\u30A3\u30EC\u30AF\u30C8\u30EA
Flatten\ directories=\u30C7\u30A3\u30EC\u30AF\u30C8\u30EA\u69CB\u9020\u3092\u7121\u8996
Optional=\u30AA\u30D7\u30B7\u30E7\u30F3
Optional=\u30AA\u30D7\u30B7\u30E7\u30F3
Fingerprint\ Artifacts=
Include\ Build\ Number=
Comment on lines +8 to +9
Copy link
Member

Choose a reason for hiding this comment

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

As above.

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<div>
Include build number in target path. Default is false.
<p>
When true, the build number of the source project will be included in the target path.
This is particularly useful when the selector is specific and the value is a permalink, e.g. lastSuccessfulBuild.
</p>
</div>
19 changes: 14 additions & 5 deletions src/main/webapp/help-flatten-optional.html
Original file line number Diff line number Diff line change
@@ -1,14 +1,23 @@
<div>
Select "Flatten directories" to ignore the directory structure of the artifacts
<p>
Select "<strong>Flatten directories</strong>" to ignore the directory structure of the artifacts
in the source project and copy all matching artifacts directly into the specified
target directory. By default the artifacts are copied in the same directory
structure as the source project.
<p/>
Select "Optional" to allow this build to continue even if no build is found
</p>
<p>
Select "<strong>Optional</strong>" to allow this build to continue even if no build is found
matching the "Which build" condition selected above, the build's workspace does
not exist or is inaccessible, or no artifacts are found matching the specified
pattern. By default this build step fails the build if no artifacts are copied.
<p/>
Select "Fingerprint Artifacts" to automatically fingerprint all artifacts
</p>
<p>
Select "<strong>Fingerprint Artifacts</strong>" to automatically fingerprint all artifacts
that are copied as part of this build step.
</p>
<p>
Select "<strong>Include Build Number</strong>" to include the source build number in the target path.
When true, the build number of the source project will be included in the target path.
This is particularly useful when the selector is specific and the value is a permalink, e.g. lastSuccessfulBuild.
</p>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -2249,7 +2249,47 @@ public void testIsValidVariableName() throws Exception {
assertFalse(CopyArtifact.isValidVariableName(" "));
assertFalse(CopyArtifact.isValidVariableName("=/?!\""));
}


@Test
public void testIncludeBuildNumberToTargetPath() throws Exception {
final Builder failureBuilder = new FailureBuilder();
final FreeStyleProject srcProject = createArtifactProject("SRC-PROJECT");

srcProject.getBuildersList().add(failureBuilder);
final FreeStyleBuild build1 = srcProject.scheduleBuild2(0).get();
rule.assertBuildStatus(Result.FAILURE, build1);

srcProject.getBuildersList().remove(failureBuilder);
final FreeStyleBuild build2 = srcProject.scheduleBuild2(0).get();
rule.assertBuildStatus(Result.SUCCESS, build2);

srcProject.getBuildersList().add(failureBuilder);
final FreeStyleBuild build3 = srcProject.scheduleBuild2(0).get();
rule.assertBuildStatus(Result.FAILURE, build3);

{
FreeStyleProject p = rule.createFreeStyleProject();
p.getBuildersList().add(CopyArtifactUtil.createCopyArtifact(
srcProject.getFullName(),
null, // parameters
new SpecificBuildSelector("lastSuccessfulBuild"),
"", // filter
"", // excludes
"", // target
false, // flatten
false, // optional
true, // fingerprintArtifacts
"", // resultVariableSuffix
true // includeBuildNumberInTargetPath
));
rule.assertBuildStatusSuccess(p.scheduleBuild2(0));

assertFalse(new FilePath(p.getWorkspace(), "1").exists());
assertTrue(new FilePath(p.getWorkspace(), "2").exists());
assertFalse(new FilePath(p.getWorkspace(), "3").exists());
}
}

@Test
public void testResultVariableSuffix() throws Exception {
FreeStyleProject srcProject = createArtifactProject("SRC-PROJECT1");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,14 @@ public static CopyArtifact createCopyArtifact(String projectName, String paramet
boolean flatten, boolean optional, boolean fingerprintArtifacts) {
return createCopyArtifact(projectName, parameters, selector, filter, excludes, target, flatten, optional, fingerprintArtifacts, null);
}


public static CopyArtifact createCopyArtifact(String projectName, String parameters, BuildSelector selector, String filter, String excludes, String target,
boolean flatten, boolean optional, boolean fingerprintArtifacts, String resultVariableSuffix) {
return createCopyArtifact(projectName, parameters, selector, filter, excludes, target, flatten, optional, fingerprintArtifacts, resultVariableSuffix, false);
}

public static CopyArtifact createCopyArtifact(String projectName, String parameters, BuildSelector selector, String filter, String excludes, String target,
boolean flatten, boolean optional, boolean fingerprintArtifacts, String resultVariableSuffix) {
boolean flatten, boolean optional, boolean fingerprintArtifacts, String resultVariableSuffix, boolean includeBuildNumberInTargetPath) {
CopyArtifact copyArtifact = new CopyArtifact(projectName);
copyArtifact.setParameters(parameters);
copyArtifact.setSelector(selector);
Expand All @@ -61,6 +66,7 @@ public static CopyArtifact createCopyArtifact(String projectName, String paramet
copyArtifact.setOptional(optional);
copyArtifact.setFingerprintArtifacts(fingerprintArtifacts);
copyArtifact.setResultVariableSuffix(resultVariableSuffix);
copyArtifact.setIncludeBuildNumberInTargetPath(includeBuildNumberInTargetPath);
return copyArtifact;
}
}