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

JENKINS-5347 Fixed - Added use commit times on files #64

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions src/main/java/hudson/scm/SubversionSCM.java
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ public class SubversionSCM extends SCM implements Serializable {

private boolean ignoreDirPropChanges;
private boolean filterChangelog;
private boolean usingCommitTimes;
Copy link
Member

Choose a reason for hiding this comment

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

IMO should be called useCommitTimes as we already have 'useUpdate' (even though that's deprecated) and the subversion and SVNKit option itself is also called 'use-commit-times'


/**
* A cache of the svn:externals (keyed by project).
Expand Down Expand Up @@ -357,15 +358,16 @@ public SubversionSCM(List<ModuleLocation> locations, WorkspaceUpdater workspaceU
public SubversionSCM(List<ModuleLocation> locations, WorkspaceUpdater workspaceUpdater,
SubversionRepositoryBrowser browser, String excludedRegions, String excludedUsers, String excludedRevprop, String excludedCommitMessages,
String includedRegions, boolean ignoreDirPropChanges) {
this(locations, workspaceUpdater, browser, excludedRegions, excludedUsers, excludedRevprop, excludedCommitMessages, includedRegions, ignoreDirPropChanges, false, null);
this(locations, workspaceUpdater, browser, excludedRegions, excludedUsers, excludedRevprop, excludedCommitMessages, includedRegions, ignoreDirPropChanges, false, null, false);
Copy link
Member

Choose a reason for hiding this comment

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

You're supposed to add another constructor for backwards compatibility instead of modifying an existing one. (This is why there are so many deprecated ones with fewer arguments)

}

@DataBoundConstructor
public SubversionSCM(List<ModuleLocation> locations, WorkspaceUpdater workspaceUpdater,
SubversionRepositoryBrowser browser, String excludedRegions, String excludedUsers,
String excludedRevprop, String excludedCommitMessages,
String includedRegions, boolean ignoreDirPropChanges, boolean filterChangelog,
List<AdditionalCredentials> additionalCredentials) {
List<AdditionalCredentials> additionalCredentials, boolean usingCommitTimes) {
this.usingCommitTimes = usingCommitTimes;
for (Iterator<ModuleLocation> itr = locations.iterator(); itr.hasNext(); ) {
ModuleLocation ml = itr.next();
String remote = Util.fixEmptyAndTrim(ml.remote);
Expand Down Expand Up @@ -674,6 +676,11 @@ public boolean isFilterChangelog() {
return filterChangelog;
}

@Exported
public boolean isUsingCommitTimes() {
return usingCommitTimes;
}

/**
* Sets the <tt>SVN_REVISION_n</tt> and <tt>SVN_URL_n</tt> environment variables during the build.
*/
Expand Down Expand Up @@ -980,6 +987,7 @@ private synchronized Map<AbstractProject, List<External>> getProjectExternalsCac
*/
private static class CheckOutTask extends UpdateTask implements FileCallable<List<External>> {
private final UpdateTask task;
private final boolean isUsingCommitTimes;

public CheckOutTask(AbstractBuild<?, ?> build, SubversionSCM parent, ModuleLocation location, Date timestamp, TaskListener listener, EnvVars env) {
this.authProvider = parent.createAuthenticationProvider(build.getParent(), location);
Expand All @@ -988,6 +996,7 @@ public CheckOutTask(AbstractBuild<?, ?> build, SubversionSCM parent, ModuleLocat
this.location = location;
this.revisions = build.getAction(RevisionParameterAction.class);
this.task = parent.getWorkspaceUpdater().createTask();
this.isUsingCommitTimes = parent.isUsingCommitTimes();
}

public Set<String> getUnauthenticatedRealms() {
Expand All @@ -998,7 +1007,12 @@ public Set<String> getUnauthenticatedRealms() {
}

public List<External> invoke(File ws, VirtualChannel channel) throws IOException {
clientManager = createClientManager(authProvider);
DefaultSVNOptions defaultSVNOptions = createDefaultSVNOptions();
if (isUsingCommitTimes) {
defaultSVNOptions.setUseCommitTimes(isUsingCommitTimes);
}

clientManager = new SvnClientManager(SVNClientManager.newInstance(defaultSVNOptions, createSvnAuthenticationManager(authProvider)));
manager = clientManager.getCore();
this.ws = ws;
try {
Expand Down
3 changes: 3 additions & 0 deletions src/main/resources/hudson/scm/SubversionSCM/config.jelly
Original file line number Diff line number Diff line change
Expand Up @@ -111,5 +111,8 @@ THE SOFTWARE.
<f:entry title="${%Filter changelog}" field="filterChangelog">
<f:checkbox />
</f:entry>
<f:entry title="${%Set check out file dates to the 'last commit time'}" field="usingCommitTimes">
<f:checkbox />
</f:entry>
</f:advanced>
</j:jelly>
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<div>
If set Jenkins will set the file timestamps to the last commit time (of each file) when doing a checkout or an update. Otherwise Jenkins will set the current date as the timestamp of each file.
<p>
Normally the working copy files have timestamps that reflect the last time they were touched by any process. This is generally convenient for most build systems as they look at timestamps as a way of deciding which files need to be recompiled.
In other situations, however, it's sometimes nice for the working copy files to have timestamps that reflect the last time they were changed in the repository. The svn export command always places these 'last-commit timestamps' on trees that it produces..
</p>
</div>
Copy link
Member

Choose a reason for hiding this comment

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

Should probably mention that this refers to the 'use-commit-times' option of subversion and optionally link to subversion red-book for more detailed info.

40 changes: 39 additions & 1 deletion src/test/java/hudson/scm/SubversionSCMTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,17 @@
package hudson.scm;

import static hudson.scm.SubversionSCM.compareSVNAuthentications;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.junit.Assert.assertThat;
import static org.jvnet.hudson.test.recipes.PresetData.DataSet.ANONYMOUS_READONLY;

import com.cloudbees.plugins.credentials.Credentials;
import com.cloudbees.plugins.credentials.CredentialsScope;
import com.cloudbees.plugins.credentials.SystemCredentialsProvider;
import com.cloudbees.plugins.credentials.domains.Domain;
import com.cloudbees.plugins.credentials.impl.UsernamePasswordCredentialsImpl;

import hudson.FilePath;
import hudson.Launcher;
import hudson.Proc;
Expand Down Expand Up @@ -71,6 +75,7 @@
import java.nio.charset.Charset;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Calendar;
import java.util.Collection;
import java.util.Collections;
import java.util.Date;
Expand Down Expand Up @@ -119,6 +124,7 @@
import com.gargoylesoftware.htmlunit.html.HtmlAnchor;
import com.gargoylesoftware.htmlunit.html.HtmlForm;
import com.gargoylesoftware.htmlunit.html.HtmlPage;

import hudson.EnvVars;
import hudson.model.EnvironmentContributor;

Expand Down Expand Up @@ -866,7 +872,7 @@ private void verifyChangelogFilter(boolean shouldFilterLog) throws Exception,
File repo = new CopyExisting(getClass().getResource("JENKINS-10449.zip")).allocate();
SubversionSCM scm = new SubversionSCM(ModuleLocation.parse(new String[]{"file://" + repo.toURI().toURL().getPath()},
new String[]{"."},null,null),
new UpdateUpdater(), null, "/z.*", "", "", "", "", false, shouldFilterLog, null);
new UpdateUpdater(), null, "/z.*", "", "", "", "", false, shouldFilterLog, null, false);

FreeStyleProject p = createFreeStyleProject(String.format("testFilterChangelog-%s", shouldFilterLog));
p.setScm(scm);
Expand Down Expand Up @@ -1738,4 +1744,36 @@ private void invokeTestPollingExternalsForFile() throws Exception {
// should detect change
assertTrue(p.poll(StreamTaskListener.fromStdout()).hasChanges());
}

public void testUseCommitTimes() throws Throwable {
// Given a subversion workspace where the commit times should be used
// When the workspace is checked out
// Then verify that the last modified time stamp of the format file is 2010-12-31

FreeStyleProject p = createFreeStyleProject();
File repo = new CopyExisting(getClass().getResource("small.zip")).allocate();
SubversionSCM scm = new SubversionSCM(ModuleLocation.parse(new String[]{"file://" + repo.toURI().toURL().getPath()},
new String[]{"."},null,null), new UpdateUpdater(), null, "/z.*", "", "", "", "", false, false, null, true);
p.setScm(scm);

FreeStyleBuild b = assertBuildStatusSuccess(p.scheduleBuild2(0, new Cause.UserIdCause()).get());
// Using long matching to prevent Timezone issues, divided by 1000 as some OS does not return the exact milliseconds
assertThat(b.getWorkspace().child("b").lastModified() / 1000, is(1293845528l));
}

public void testNotUseCommitTimes() throws Throwable {
// Given a subversion workspace where the commit times should NOT be used
// When the workspace is checked out
// Then verify that the last modified time stamp of the format file NOT is 2011-01-01

FreeStyleProject p = createFreeStyleProject();
File repo = new CopyExisting(getClass().getResource("small.zip")).allocate();
SubversionSCM scm = new SubversionSCM(ModuleLocation.parse(new String[]{"file://" + repo.toURI().toURL().getPath()},
new String[]{"."},null,null), new UpdateUpdater(), null, "/z.*", "", "", "", "", false, false, null, false);
p.setScm(scm);

FreeStyleBuild b = assertBuildStatusSuccess(p.scheduleBuild2(0, new Cause.UserIdCause()).get());
// Using long matching to prevent Timezone issues, divided by 1000 as some OS does not return the exact milliseconds
assertThat(b.getWorkspace().child("b").lastModified() / 1000, not(is(1293845528l)));
}
}