You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/05/22 13:39:23 UTC
[GitHub] [maven-scm] G-Ork opened a new pull request, #144: [SCM-925] Implement RemoveCommand in maven-scm-provider-jgit
G-Ork opened a new pull request, #144:
URL: https://github.com/apache/maven-scm/pull/144
Fourth try to fix [SCM-925]
- Implement remove command for JGit provider
- Write TCK for remove command.
- No other upgrades (Java 1.8 or JGit)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] kwin commented on a diff in pull request #144: [SCM-925] Implement RemoveCommand in maven-scm-provider-jgit
Posted by GitBox <gi...@apache.org>.
kwin commented on code in PR #144:
URL: https://github.com/apache/maven-scm/pull/144#discussion_r927551052
##########
maven-scm-providers/maven-scm-providers-git/maven-scm-provider-jgit/src/main/java/org/apache/maven/scm/provider/git/jgit/command/JGitUtils.java:
##########
@@ -576,8 +576,8 @@ public static List<ScmFile> removeAllFiles( Git git, ScmFileSet fileSet )
// really tracked
for ( Iterator<File> itfl = fileSet.getFileList().iterator(); itfl.hasNext(); )
{
- String path = FilenameUtils.normalizeFilename( relativize( baseUri, itfl.next() ) );
- if ( path.equals( FilenameUtils.normalizeFilename( scmFile.getPath() ) ) )
+ String path = relativize( baseUri, itfl.next() );
+ if ( path.equals( scmFile.getPath() ) )
Review Comment:
this comparison is still wrong, IMHO you first need to make `itfl.next()` absolute and then relativize against `workingCopyRootUri`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] G-Ork commented on pull request #144: [SCM-925] Implement RemoveCommand in maven-scm-provider-jgit
Posted by GitBox <gi...@apache.org>.
G-Ork commented on PR #144:
URL: https://github.com/apache/maven-scm/pull/144#issuecomment-1133928593
I did not try any back ports. Where do i find the code base for 1.x? The tag i tried are broken so i lost patience.
There could only be two possible problems in back porting:
1. No Java 1.8 (NIO) Support. I used NIO in test because org.codehaus.plexus.util.FileUtils.fileAppend(String, String) was deprecated. Could easyly be changed.
2. JGIT does not support remove in the older verison.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] michael-o commented on pull request #144: [SCM-925] Implement RemoveCommand in maven-scm-provider-jgit
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #144:
URL: https://github.com/apache/maven-scm/pull/144#issuecomment-1133952425
> I did not try any back ports. Where do i find the code base for 1.x? The tag i tried are broken so i lost patience. There could only be two possible problems in back porting:
>
> 1. No Java 1.8 (NIO) Support. I used NIO in test because org.codehaus.plexus.util.FileUtils.fileAppend(String, String) was deprecated. Could easyly be changed.
Look into the `maven-scm-1.x` branch. It is still Java 7. If you want your change with Maven SCM 1.13.0 delivered then use Plexus Utils, the deprecation is OK for now. We will likely update all in one go.
> 2. JGIT does not support remove in the older verison.
The currently used JGit version does not support what you need?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] michael-o commented on pull request #144: [SCM-925] Implement RemoveCommand in maven-scm-provider-jgit
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #144:
URL: https://github.com/apache/maven-scm/pull/144#issuecomment-1142537483
@G-Ork Please have a look at my comments. I want to finalize this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] michael-o commented on pull request #144: [SCM-925] Implement RemoveCommand in maven-scm-provider-jgit
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #144:
URL: https://github.com/apache/maven-scm/pull/144#issuecomment-1135089606
> Yes i understand (now) what revision you address. I first have to setup an environment to be able to compile this branch. Is there a good reason not using the toolchain plugin ? It feel like Indiana Jones to dig out an java 1.4 JDK for checking correct back port.
No magic required, just switch and build. It will work.
> Is the back-port mandatory for this PR?
No, I will leave this decision totally up to you.
> Can we separate this in another PR?
Yes, if you decide to back port.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] michael-o commented on pull request #144: [SCM-925] Implement RemoveCommand in maven-scm-provider-jgit
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #144:
URL: https://github.com/apache/maven-scm/pull/144#issuecomment-1192469151
The gitexe test is incomplete as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] michael-o commented on pull request #144: [SCM-925] Implement RemoveCommand in maven-scm-provider-jgit
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #144:
URL: https://github.com/apache/maven-scm/pull/144#issuecomment-1192388135
@kwin Please review. I have rebased and cleaned up. Also check last commit, simplification, but am not 100% sure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] kwin commented on a diff in pull request #144: [SCM-925] Implement RemoveCommand in maven-scm-provider-jgit
Posted by GitBox <gi...@apache.org>.
kwin commented on code in PR #144:
URL: https://github.com/apache/maven-scm/pull/144#discussion_r927538093
##########
maven-scm-providers/maven-scm-providers-git/maven-scm-provider-gittest/src/main/java/org/apache/maven/scm/provider/git/command/remove/GitRemoveCommandTckTest.java:
##########
@@ -0,0 +1,85 @@
+package org.apache.maven.scm.provider.git.command.remove;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.io.File;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.StandardOpenOption;
+import java.util.Arrays;
+
+import org.apache.maven.scm.ScmFileSet;
+import org.apache.maven.scm.ScmTckTestCase;
+import org.apache.maven.scm.command.remove.RemoveScmResult;
+import org.apache.maven.scm.provider.git.GitScmTestUtils;
+
+/**
+ * This test tests the remove command.
+ *
+ * @author Georg Tsakumagos
+ */
+public abstract class GitRemoveCommandTckTest
+ extends ScmTckTestCase
+{
+ /** {@inheritDoc} */
+ public void initRepo()
+ throws Exception
+ {
+ GitScmTestUtils.initRepo( "src/test/resources/repository/", getRepositoryRoot(), getWorkingDirectory() );
+ }
+
+ public void testCommandRemoveWithFile()
+ throws Exception
+ {
+ File toBeRemoved = new File( getWorkingDirectory().getAbsolutePath() + File.separator + "toto.xml" );
+
+ Files.write( toBeRemoved.getAbsoluteFile().toPath(),
Review Comment:
Right, then maybe simplify to
```suggestion
Files.write( toBeRemoved.toPath(), Collections.singletonList("data"), StandardCharsets.ASCII, StandardOpenOption.CREATE )
```
##########
maven-scm-providers/maven-scm-providers-git/maven-scm-provider-gittest/src/main/java/org/apache/maven/scm/provider/git/command/remove/GitRemoveCommandTckTest.java:
##########
@@ -0,0 +1,85 @@
+package org.apache.maven.scm.provider.git.command.remove;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.io.File;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.StandardOpenOption;
+import java.util.Arrays;
+
+import org.apache.maven.scm.ScmFileSet;
+import org.apache.maven.scm.ScmTckTestCase;
+import org.apache.maven.scm.command.remove.RemoveScmResult;
+import org.apache.maven.scm.provider.git.GitScmTestUtils;
+
+/**
+ * This test tests the remove command.
+ *
+ * @author Georg Tsakumagos
+ */
+public abstract class GitRemoveCommandTckTest
+ extends ScmTckTestCase
+{
+ /** {@inheritDoc} */
+ public void initRepo()
+ throws Exception
+ {
+ GitScmTestUtils.initRepo( "src/test/resources/repository/", getRepositoryRoot(), getWorkingDirectory() );
+ }
+
+ public void testCommandRemoveWithFile()
+ throws Exception
+ {
+ File toBeRemoved = new File( getWorkingDirectory().getAbsolutePath() + File.separator + "toto.xml" );
Review Comment:
```suggestion
File toBeRemoved = new File( getWorkingDirectory(), "toto.xml" );
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] michael-o commented on pull request #144: [SCM-925] Implement RemoveCommand in maven-scm-provider-jgit
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #144:
URL: https://github.com/apache/maven-scm/pull/144#issuecomment-1133911062
Looks much better now, will happily review.
Did you try, does this apply cleanly to 1.x? If yes, I'd back port and deliver with upcoming 1.13.0.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] michael-o commented on pull request #144: [SCM-925] Implement RemoveCommand in maven-scm-provider-jgit
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #144:
URL: https://github.com/apache/maven-scm/pull/144#issuecomment-1186296375
Still waiting about the duplicate addition of the file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] michael-o commented on pull request #144: [SCM-925] Implement RemoveCommand in maven-scm-provider-jgit
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #144:
URL: https://github.com/apache/maven-scm/pull/144#issuecomment-1176331937
@G-Ork Still waiting for finalization and this will land in M2.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] asfgit closed pull request #144: [SCM-925] Implement RemoveCommand in maven-scm-provider-jgit
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #144: [SCM-925] Implement RemoveCommand in maven-scm-provider-jgit
URL: https://github.com/apache/maven-scm/pull/144
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] G-Ork commented on pull request #144: [SCM-925] Implement RemoveCommand in maven-scm-provider-jgit
Posted by GitBox <gi...@apache.org>.
G-Ork commented on PR #144:
URL: https://github.com/apache/maven-scm/pull/144#issuecomment-1134188602
I just enumerate the theoretical edges. I did not verify it. For myself i do not see any advantages to back port this to an very old maven. But you may have better understanding how many may use Maven 1 and JGit. If you could give good reason to port i will help.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] michael-o commented on pull request #144: [SCM-925] Implement RemoveCommand in maven-scm-provider-jgit
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #144:
URL: https://github.com/apache/maven-scm/pull/144#issuecomment-1192459640
I think the TCK test is logically flawed. It removes files and dirs:
* File has never been added to SCM
* You cannot remove dirs with Git if there is no content. Not visible to Git
* It is a copy of `org.apache.maven.scm.provider.git.gitexe.command.remove.GitRemoveCommandTest` which checks the generated CLI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] michael-o commented on pull request #144: [SCM-925] Implement RemoveCommand in maven-scm-provider-jgit
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #144:
URL: https://github.com/apache/maven-scm/pull/144#issuecomment-1192461554
@kwin Applied most of your comments
ยด
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] michael-o commented on a diff in pull request #144: [SCM-925] Implement RemoveCommand in maven-scm-provider-jgit
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #144:
URL: https://github.com/apache/maven-scm/pull/144#discussion_r924857373
##########
maven-scm-providers/maven-scm-providers-git/maven-scm-provider-jgit/src/main/java/org/apache/maven/scm/provider/git/jgit/command/JGitUtils.java:
##########
@@ -461,5 +462,60 @@ public RevFilter clone()
return revs;
}
}
+
+ /**
+ * Remove all files in the given fileSet to the repository.
+ *
+ * @param git the repo to remove the files from
+ * @param fileSet the set of files within the workspace, the files are removed
+ * relative to the basedir of this fileset
+ * @return a list of removed files
+ * @throws GitAPIException
+ * @throws NoFilepatternException
+ */
+ public static List<ScmFile> removeAllFiles( Git git, ScmFileSet fileSet )
+ throws GitAPIException, NoFilepatternException
+ {
+ URI baseUri = fileSet.getBasedir().toURI();
+ RmCommand remove = git.rm();
+ for ( File file : fileSet.getFileList() )
+ {
+ if ( !file.isAbsolute() )
+ {
+ file = new File( fileSet.getBasedir().getPath(), file.getPath() );
+ }
+
+ if ( file.exists() )
+ {
+ String path = relativize( baseUri, file );
+ remove.addFilepattern( path );
+ remove.addFilepattern( file.getAbsolutePath() );
Review Comment:
@kwin Maybe you have an explanation for this...?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] michael-o commented on a diff in pull request #144: [SCM-925] Implement RemoveCommand in maven-scm-provider-jgit
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #144:
URL: https://github.com/apache/maven-scm/pull/144#discussion_r927551797
##########
maven-scm-providers/maven-scm-providers-git/maven-scm-provider-jgit/src/main/java/org/apache/maven/scm/provider/git/jgit/command/JGitUtils.java:
##########
@@ -576,8 +576,8 @@ public static List<ScmFile> removeAllFiles( Git git, ScmFileSet fileSet )
// really tracked
for ( Iterator<File> itfl = fileSet.getFileList().iterator(); itfl.hasNext(); )
{
- String path = FilenameUtils.normalizeFilename( relativize( baseUri, itfl.next() ) );
- if ( path.equals( FilenameUtils.normalizeFilename( scmFile.getPath() ) ) )
+ String path = relativize( baseUri, itfl.next() );
+ if ( path.equals( scmFile.getPath() ) )
Review Comment:
Correct. Please fork and provide a PR on top. You know better what to do already.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] kwin commented on pull request #144: [SCM-925] Implement RemoveCommand in maven-scm-provider-jgit
Posted by GitBox <gi...@apache.org>.
kwin commented on PR #144:
URL: https://github.com/apache/maven-scm/pull/144#issuecomment-1189890572
@michael-o I would recommend to first clarify https://github.com/apache/maven-scm/pull/152#issuecomment-1178024189 as this command also returns file paths.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] G-Ork commented on pull request #144: [SCM-925] Implement RemoveCommand in maven-scm-provider-jgit
Posted by GitBox <gi...@apache.org>.
G-Ork commented on PR #144:
URL: https://github.com/apache/maven-scm/pull/144#issuecomment-1135084062
Yes i understand (now) what revision you address. I first have to setup an environment to be able to compile this branch. Is there a good reason not using the toolchain plugin ? It feel like Indiana Jones to dig out an java 1.4 JDK for checking correct back port.
Is the back-port mandatory for this PR?
Can we separate this in another PR?
I will free time time at Wednesday evening.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] kwin commented on a diff in pull request #144: [SCM-925] Implement RemoveCommand in maven-scm-provider-jgit
Posted by GitBox <gi...@apache.org>.
kwin commented on code in PR #144:
URL: https://github.com/apache/maven-scm/pull/144#discussion_r925229583
##########
maven-scm-providers/maven-scm-providers-git/maven-scm-provider-jgit/src/main/java/org/apache/maven/scm/provider/git/jgit/command/JGitUtils.java:
##########
@@ -461,5 +462,60 @@ public RevFilter clone()
return revs;
}
}
+
+ /**
+ * Remove all files in the given fileSet to the repository.
+ *
+ * @param git the repo to remove the files from
+ * @param fileSet the set of files within the workspace, the files are removed
+ * relative to the basedir of this fileset
+ * @return a list of removed files
+ * @throws GitAPIException
+ * @throws NoFilepatternException
+ */
+ public static List<ScmFile> removeAllFiles( Git git, ScmFileSet fileSet )
+ throws GitAPIException, NoFilepatternException
+ {
+ URI baseUri = fileSet.getBasedir().toURI();
+ RmCommand remove = git.rm();
+ for ( File file : fileSet.getFileList() )
+ {
+ if ( !file.isAbsolute() )
+ {
+ file = new File( fileSet.getBasedir().getPath(), file.getPath() );
+ }
+
+ if ( file.exists() )
+ {
+ String path = relativize( baseUri, file );
+ remove.addFilepattern( path );
+ remove.addFilepattern( file.getAbsolutePath() );
Review Comment:
RmCommand needs repository relative paths with forward slashes (https://archive.eclipse.org/jgit/site/5.0.1.201806211838-r/apidocs/org/eclipse/jgit/api/RmCommand.html#addFilepattern-java.lang.String-), therefore IMHO we need:
```
URI workingCopyRootUri = git.getRepository().getWorkTree().toURI();
String path = FilenameUtils.normalizeFilename( relativize( workingCopyRootUri, file ) );
remove.addFilepattern( path );
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] michael-o commented on pull request #144: [SCM-925] Implement RemoveCommand in maven-scm-provider-jgit
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #144:
URL: https://github.com/apache/maven-scm/pull/144#issuecomment-1192767254
> I may find some time next week to look into that. If you want to release M2 earlier just defer this.
Thank you. I will defer and leave you and @G-Ork enough time to crunch on this. Especially because we have `gitexe` with works as an alternative. I will continue with the release today.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] michael-o commented on pull request #144: [SCM-925] Implement RemoveCommand in maven-scm-provider-jgit
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #144:
URL: https://github.com/apache/maven-scm/pull/144#issuecomment-1135010487
@G-Ork I am talking about https://github.com/apache/maven-scm/tree/maven-scm-1.x.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] michael-o commented on a diff in pull request #144: [SCM-925] Implement RemoveCommand in maven-scm-provider-jgit
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #144:
URL: https://github.com/apache/maven-scm/pull/144#discussion_r882188679
##########
maven-scm-providers/maven-scm-providers-git/maven-scm-provider-jgit/src/main/java/org/apache/maven/scm/provider/git/jgit/command/remove/JGitRemoveCommand.java:
##########
@@ -0,0 +1,81 @@
+package org.apache.maven.scm.provider.git.jgit.command.remove;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.List;
+
+import org.apache.maven.scm.ScmException;
+import org.apache.maven.scm.ScmFile;
+import org.apache.maven.scm.ScmFileSet;
+import org.apache.maven.scm.ScmResult;
+import org.apache.maven.scm.command.remove.AbstractRemoveCommand;
+import org.apache.maven.scm.command.remove.RemoveScmResult;
+import org.apache.maven.scm.provider.ScmProviderRepository;
+import org.apache.maven.scm.provider.git.command.GitCommand;
+import org.apache.maven.scm.provider.git.jgit.command.JGitUtils;
+import org.eclipse.jgit.api.Git;
+
+/**
+ * @author Georg Tsakumagos
+ * @since 2.0.0-M1
+ */
+public class JGitRemoveCommand
+ extends AbstractRemoveCommand
+ implements GitCommand
+{
+
+ @Override
+ protected ScmResult executeRemoveCommand( ScmProviderRepository repository, ScmFileSet fileSet, String message )
+ throws ScmException
+ {
+
+ if ( fileSet.getFileList().isEmpty() )
+ {
+ throw new ScmException( "You must provide at least one file/directory to remove" );
+ }
+ Git git = null;
+ try
+ {
+ git = JGitUtils.openRepo( fileSet.getBasedir() );
+
+ List<ScmFile> removedFiles = JGitUtils.removeAllFiles( git, fileSet );
+
+ if ( logger.isDebugEnabled() )
+ {
+ for ( ScmFile scmFile : removedFiles )
+ {
+ logger.info( "removed file: " + scmFile );
Review Comment:
Removed
##########
maven-scm-providers/maven-scm-providers-git/maven-scm-provider-jgit/src/main/java/org/apache/maven/scm/provider/git/jgit/command/JGitUtils.java:
##########
@@ -461,5 +462,60 @@ public RevFilter clone()
return revs;
}
}
+
+ /**
+ * Remove all files in the given fileSet to the repository.
+ *
+ * @param git the repo to remove the files from
+ * @param fileSet the set of files within the workspace, the files are removed
+ * relative to the basedir of this fileset
+ * @return a list of removed files
+ * @throws GitAPIException
+ * @throws NoFilepatternException
+ */
+ public static List<ScmFile> removeAllFiles( Git git, ScmFileSet fileSet )
+ throws GitAPIException, NoFilepatternException
+ {
+ URI baseUri = fileSet.getBasedir().toURI();
+ RmCommand remove = git.rm();
+ for ( File file : fileSet.getFileList() )
+ {
+ if ( !file.isAbsolute() )
+ {
+ file = new File( fileSet.getBasedir().getPath(), file.getPath() );
+ }
+
+ if ( file.exists() )
+ {
+ String path = relativize( baseUri, file );
+ remove.addFilepattern( path );
+ remove.addFilepattern( file.getAbsolutePath() );
+ }
+ }
+ remove.call();
+
+ Status status = git.status().call();
+
+ Set<String> allInIndex = new HashSet<String>();
+ allInIndex.addAll( status.getRemoved() );
+ List<ScmFile> removedFiles = new ArrayList<ScmFile>( allInIndex.size() );
+
+ // rewrite all detected files to now have status 'checked_in'
+ for ( String entry : allInIndex )
+ {
+ ScmFile scmfile = new ScmFile( entry, ScmFileStatus.DELETED );
Review Comment:
`scmFile`
##########
maven-scm-providers/maven-scm-providers-git/maven-scm-provider-gittest/src/main/java/org/apache/maven/scm/provider/git/command/remove/RemoveCommandTckTest.java:
##########
@@ -0,0 +1,106 @@
+package org.apache.maven.scm.provider.git.command.remove;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.StandardOpenOption;
+import java.util.Arrays;
+
+import org.apache.maven.scm.ScmFileSet;
+import org.apache.maven.scm.ScmTckTestCase;
+import org.apache.maven.scm.command.remove.RemoveScmResult;
+import org.apache.maven.scm.provider.git.GitScmTestUtils;
+import org.codehaus.plexus.util.FileUtils;
+
+/**
+ * This test tests the remove command.
+ *
+ * @author Georg Tsakumagos
+ */
+public abstract class RemoveCommandTckTest
+ extends ScmTckTestCase
+{
+ /** {@inheritDoc} */
+ public void initRepo()
+ throws Exception
+ {
+ GitScmTestUtils.initRepo( "src/test/resources/repository/", getRepositoryRoot(), getWorkingDirectory() );
+ }
+
+ public void testCommandRemoveWithFile()
+ throws Exception
+ {
+ File workingDirectory = createTempDirectory();
+
+ File toBeRemoved = new File( workingDirectory.getAbsolutePath() + File.separator + "toto.xml" );
+
+ Files.write(toBeRemoved.getAbsoluteFile().toPath(), "data".getBytes(StandardCharsets.UTF_8), StandardOpenOption.APPEND, StandardOpenOption.CREATE );
Review Comment:
Space around parens
##########
maven-scm-providers/maven-scm-providers-git/maven-scm-provider-jgit/src/main/java/org/apache/maven/scm/provider/git/jgit/command/remove/JGitRemoveCommand.java:
##########
@@ -0,0 +1,81 @@
+package org.apache.maven.scm.provider.git.jgit.command.remove;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.List;
+
+import org.apache.maven.scm.ScmException;
+import org.apache.maven.scm.ScmFile;
+import org.apache.maven.scm.ScmFileSet;
+import org.apache.maven.scm.ScmResult;
+import org.apache.maven.scm.command.remove.AbstractRemoveCommand;
+import org.apache.maven.scm.command.remove.RemoveScmResult;
+import org.apache.maven.scm.provider.ScmProviderRepository;
+import org.apache.maven.scm.provider.git.command.GitCommand;
+import org.apache.maven.scm.provider.git.jgit.command.JGitUtils;
+import org.eclipse.jgit.api.Git;
+
+/**
+ * @author Georg Tsakumagos
+ * @since 2.0.0-M1
Review Comment:
M2
##########
maven-scm-providers/maven-scm-providers-git/maven-scm-provider-gittest/src/main/java/org/apache/maven/scm/provider/git/command/remove/RemoveCommandTckTest.java:
##########
@@ -0,0 +1,106 @@
+package org.apache.maven.scm.provider.git.command.remove;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.StandardOpenOption;
+import java.util.Arrays;
+
+import org.apache.maven.scm.ScmFileSet;
+import org.apache.maven.scm.ScmTckTestCase;
+import org.apache.maven.scm.command.remove.RemoveScmResult;
+import org.apache.maven.scm.provider.git.GitScmTestUtils;
+import org.codehaus.plexus.util.FileUtils;
+
+/**
+ * This test tests the remove command.
+ *
+ * @author Georg Tsakumagos
+ */
+public abstract class RemoveCommandTckTest
+ extends ScmTckTestCase
+{
+ /** {@inheritDoc} */
+ public void initRepo()
+ throws Exception
+ {
+ GitScmTestUtils.initRepo( "src/test/resources/repository/", getRepositoryRoot(), getWorkingDirectory() );
+ }
+
+ public void testCommandRemoveWithFile()
+ throws Exception
+ {
+ File workingDirectory = createTempDirectory();
+
+ File toBeRemoved = new File( workingDirectory.getAbsolutePath() + File.separator + "toto.xml" );
+
+ Files.write(toBeRemoved.getAbsoluteFile().toPath(), "data".getBytes(StandardCharsets.UTF_8), StandardOpenOption.APPEND, StandardOpenOption.CREATE );
+
+ ScmFileSet fileSet = new ScmFileSet( getWorkingCopy(), toBeRemoved );
+ RemoveScmResult removeResult = getScmManager().remove( getScmRepository(), fileSet, getBasedir() );
+ assertResultIsSuccess( removeResult );
+
+ FileUtils.deleteDirectory( workingDirectory );
+ }
+
+ public void testCommandRemoveWithDirectory()
+ throws Exception
+ {
+ File workingDirectory = createTempDirectory();
+
+ File toBeRemoved = new File( workingDirectory.getAbsolutePath() + File.separator + "toto" );
+ toBeRemoved.mkdir();
+
+ ScmFileSet fileSet = new ScmFileSet( getWorkingCopy(), toBeRemoved );
+ RemoveScmResult removeResult = getScmManager().remove( getScmRepository(), fileSet, getBasedir() );
+ assertResultIsSuccess( removeResult );
+
+ FileUtils.deleteDirectory( workingDirectory );
+ }
+
+ public void testCommandRemoveWithTwoDirectory()
Review Comment:
TwoDirectories
##########
maven-scm-providers/maven-scm-providers-git/maven-scm-provider-jgit/src/main/java/org/apache/maven/scm/provider/git/jgit/command/JGitUtils.java:
##########
@@ -461,5 +462,60 @@ public RevFilter clone()
return revs;
}
}
+
+ /**
+ * Remove all files in the given fileSet to the repository.
+ *
+ * @param git the repo to remove the files from
+ * @param fileSet the set of files within the workspace, the files are removed
+ * relative to the basedir of this fileset
+ * @return a list of removed files
+ * @throws GitAPIException
+ * @throws NoFilepatternException
+ */
+ public static List<ScmFile> removeAllFiles( Git git, ScmFileSet fileSet )
+ throws GitAPIException, NoFilepatternException
+ {
+ URI baseUri = fileSet.getBasedir().toURI();
+ RmCommand remove = git.rm();
+ for ( File file : fileSet.getFileList() )
+ {
+ if ( !file.isAbsolute() )
+ {
+ file = new File( fileSet.getBasedir().getPath(), file.getPath() );
+ }
+
+ if ( file.exists() )
+ {
+ String path = relativize( baseUri, file );
+ remove.addFilepattern( path );
+ remove.addFilepattern( file.getAbsolutePath() );
Review Comment:
Why is this added twice?
##########
maven-scm-providers/maven-scm-providers-git/maven-scm-provider-gittest/src/main/java/org/apache/maven/scm/provider/git/command/remove/RemoveCommandTckTest.java:
##########
@@ -0,0 +1,106 @@
+package org.apache.maven.scm.provider.git.command.remove;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.StandardOpenOption;
+import java.util.Arrays;
+
+import org.apache.maven.scm.ScmFileSet;
+import org.apache.maven.scm.ScmTckTestCase;
+import org.apache.maven.scm.command.remove.RemoveScmResult;
+import org.apache.maven.scm.provider.git.GitScmTestUtils;
+import org.codehaus.plexus.util.FileUtils;
+
+/**
+ * This test tests the remove command.
+ *
+ * @author Georg Tsakumagos
+ */
+public abstract class RemoveCommandTckTest
+ extends ScmTckTestCase
+{
+ /** {@inheritDoc} */
+ public void initRepo()
+ throws Exception
+ {
+ GitScmTestUtils.initRepo( "src/test/resources/repository/", getRepositoryRoot(), getWorkingDirectory() );
+ }
+
+ public void testCommandRemoveWithFile()
+ throws Exception
+ {
+ File workingDirectory = createTempDirectory();
+
+ File toBeRemoved = new File( workingDirectory.getAbsolutePath() + File.separator + "toto.xml" );
+
+ Files.write(toBeRemoved.getAbsoluteFile().toPath(), "data".getBytes(StandardCharsets.UTF_8), StandardOpenOption.APPEND, StandardOpenOption.CREATE );
+
+ ScmFileSet fileSet = new ScmFileSet( getWorkingCopy(), toBeRemoved );
+ RemoveScmResult removeResult = getScmManager().remove( getScmRepository(), fileSet, getBasedir() );
+ assertResultIsSuccess( removeResult );
+
+ FileUtils.deleteDirectory( workingDirectory );
+ }
+
+ public void testCommandRemoveWithDirectory()
+ throws Exception
+ {
+ File workingDirectory = createTempDirectory();
+
+ File toBeRemoved = new File( workingDirectory.getAbsolutePath() + File.separator + "toto" );
+ toBeRemoved.mkdir();
+
+ ScmFileSet fileSet = new ScmFileSet( getWorkingCopy(), toBeRemoved );
+ RemoveScmResult removeResult = getScmManager().remove( getScmRepository(), fileSet, getBasedir() );
+ assertResultIsSuccess( removeResult );
+
+ FileUtils.deleteDirectory( workingDirectory );
+ }
+
+ public void testCommandRemoveWithTwoDirectory()
+ throws Exception
+ {
+ File workingDirectory = createTempDirectory();
+
+ File toBeRemoved1 = new File( workingDirectory.getAbsolutePath() + File.separator + "toto" );
+ toBeRemoved1.mkdir();
+
+ File toBeRemoved2 = new File( workingDirectory.getAbsolutePath() + File.separator + "tata" );
+ toBeRemoved2.mkdir();
+
+ ScmFileSet fileSet = new ScmFileSet( getWorkingCopy(), Arrays.asList( toBeRemoved1, toBeRemoved2 ) );
+ RemoveScmResult removeResult = getScmManager().remove( getScmRepository(), fileSet, getBasedir() );
+ assertResultIsSuccess( removeResult );
+ FileUtils.deleteDirectory( workingDirectory );
+ }
+
+ private File createTempDirectory()
+ throws IOException
+ {
+ File dir = File.createTempFile( "gitexe", "test" );
Review Comment:
This is jgit, no?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] michael-o commented on a diff in pull request #144: [SCM-925] Implement RemoveCommand in maven-scm-provider-jgit
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #144:
URL: https://github.com/apache/maven-scm/pull/144#discussion_r927531775
##########
maven-scm-providers/maven-scm-providers-git/maven-scm-provider-gittest/src/main/java/org/apache/maven/scm/provider/git/command/remove/GitRemoveCommandTckTest.java:
##########
@@ -0,0 +1,85 @@
+package org.apache.maven.scm.provider.git.command.remove;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.io.File;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.StandardOpenOption;
+import java.util.Arrays;
+
+import org.apache.maven.scm.ScmFileSet;
+import org.apache.maven.scm.ScmTckTestCase;
+import org.apache.maven.scm.command.remove.RemoveScmResult;
+import org.apache.maven.scm.provider.git.GitScmTestUtils;
+
+/**
+ * This test tests the remove command.
+ *
+ * @author Georg Tsakumagos
+ */
+public abstract class GitRemoveCommandTckTest
+ extends ScmTckTestCase
+{
+ /** {@inheritDoc} */
+ public void initRepo()
+ throws Exception
+ {
+ GitScmTestUtils.initRepo( "src/test/resources/repository/", getRepositoryRoot(), getWorkingDirectory() );
+ }
+
+ public void testCommandRemoveWithFile()
+ throws Exception
+ {
+ File toBeRemoved = new File( getWorkingDirectory().getAbsolutePath() + File.separator + "toto.xml" );
+
+ Files.write( toBeRemoved.getAbsoluteFile().toPath(),
Review Comment:
Does it matter, because Git tracks content, not files?!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] kwin commented on pull request #144: [SCM-925] Implement RemoveCommand in maven-scm-provider-jgit
Posted by GitBox <gi...@apache.org>.
kwin commented on PR #144:
URL: https://github.com/apache/maven-scm/pull/144#issuecomment-1193181758
@michael-o You find my refinement in #156.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] kwin commented on a diff in pull request #144: [SCM-925] Implement RemoveCommand in maven-scm-provider-jgit
Posted by GitBox <gi...@apache.org>.
kwin commented on code in PR #144:
URL: https://github.com/apache/maven-scm/pull/144#discussion_r927524789
##########
maven-scm-providers/maven-scm-providers-git/maven-scm-provider-gittest/src/main/java/org/apache/maven/scm/provider/git/command/remove/GitRemoveCommandTckTest.java:
##########
@@ -0,0 +1,85 @@
+package org.apache.maven.scm.provider.git.command.remove;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.io.File;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.StandardOpenOption;
+import java.util.Arrays;
+
+import org.apache.maven.scm.ScmFileSet;
+import org.apache.maven.scm.ScmTckTestCase;
+import org.apache.maven.scm.command.remove.RemoveScmResult;
+import org.apache.maven.scm.provider.git.GitScmTestUtils;
+
+/**
+ * This test tests the remove command.
+ *
+ * @author Georg Tsakumagos
+ */
+public abstract class GitRemoveCommandTckTest
+ extends ScmTckTestCase
+{
+ /** {@inheritDoc} */
+ public void initRepo()
+ throws Exception
+ {
+ GitScmTestUtils.initRepo( "src/test/resources/repository/", getRepositoryRoot(), getWorkingDirectory() );
+ }
+
+ public void testCommandRemoveWithFile()
+ throws Exception
+ {
+ File toBeRemoved = new File( getWorkingDirectory().getAbsolutePath() + File.separator + "toto.xml" );
+
+ Files.write( toBeRemoved.getAbsoluteFile().toPath(),
Review Comment:
why not using an empty file created with https://docs.oracle.com/javase/7/docs/api/java/nio/file/Files.html#createFile(java.nio.file.Path,%20java.nio.file.attribute.FileAttribute...), the content does not matter in this case.
##########
maven-scm-providers/maven-scm-providers-git/maven-scm-provider-gittest/src/main/java/org/apache/maven/scm/provider/git/command/remove/GitRemoveCommandTckTest.java:
##########
@@ -0,0 +1,85 @@
+package org.apache.maven.scm.provider.git.command.remove;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.io.File;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.StandardOpenOption;
+import java.util.Arrays;
+
+import org.apache.maven.scm.ScmFileSet;
+import org.apache.maven.scm.ScmTckTestCase;
+import org.apache.maven.scm.command.remove.RemoveScmResult;
+import org.apache.maven.scm.provider.git.GitScmTestUtils;
+
+/**
+ * This test tests the remove command.
+ *
+ * @author Georg Tsakumagos
+ */
+public abstract class GitRemoveCommandTckTest
+ extends ScmTckTestCase
+{
+ /** {@inheritDoc} */
+ public void initRepo()
+ throws Exception
+ {
+ GitScmTestUtils.initRepo( "src/test/resources/repository/", getRepositoryRoot(), getWorkingDirectory() );
+ }
+
+ public void testCommandRemoveWithFile()
Review Comment:
I think we use JUnit4, so a `@Test` annotation is necessary for m-surefire-p to pick this up.
##########
maven-scm-providers/maven-scm-providers-git/maven-scm-provider-jgit/src/main/java/org/apache/maven/scm/provider/git/jgit/command/JGitUtils.java:
##########
@@ -527,4 +529,61 @@ public static List<String> getTags( Repository repo, RevCommit commit ) throws I
}
return result;
}
+
+ /**
+ * Remove all files in the given fileSet to the repository.
+ *
+ * @param git the repo to remove the files from
+ * @param fileSet the set of files within the workspace, the files are removed
+ * relative to the basedir of this fileset
+ * @return a list of removed files
+ * @throws GitAPIException
+ * @throws NoFilepatternException
+ */
+ public static List<ScmFile> removeAllFiles( Git git, ScmFileSet fileSet )
+ throws GitAPIException, NoFilepatternException
+ {
+ URI baseUri = fileSet.getBasedir().toURI();
+ RmCommand remove = git.rm();
+ for ( File file : fileSet.getFileList() )
+ {
+ if ( !file.isAbsolute() )
+ {
+ file = new File( fileSet.getBasedir().getPath(), file.getPath() );
+ }
+
+ if ( file.exists() )
+ {
+ URI workingCopyRootUri = git.getRepository().getWorkTree().toURI();
+ String path = FilenameUtils.normalizeFilename( relativize( workingCopyRootUri, file ) );
Review Comment:
I think you figured out in the other PR that normalizeFilename is redundant in this case, as relativize always returns a path with forward slashes.
##########
maven-scm-providers/maven-scm-providers-git/maven-scm-provider-jgit/src/main/java/org/apache/maven/scm/provider/git/jgit/command/JGitUtils.java:
##########
@@ -527,4 +529,61 @@ public static List<String> getTags( Repository repo, RevCommit commit ) throws I
}
return result;
}
+
+ /**
+ * Remove all files in the given fileSet to the repository.
+ *
+ * @param git the repo to remove the files from
+ * @param fileSet the set of files within the workspace, the files are removed
+ * relative to the basedir of this fileset
+ * @return a list of removed files
+ * @throws GitAPIException
+ * @throws NoFilepatternException
+ */
+ public static List<ScmFile> removeAllFiles( Git git, ScmFileSet fileSet )
+ throws GitAPIException, NoFilepatternException
+ {
+ URI baseUri = fileSet.getBasedir().toURI();
+ RmCommand remove = git.rm();
+ for ( File file : fileSet.getFileList() )
+ {
+ if ( !file.isAbsolute() )
+ {
+ file = new File( fileSet.getBasedir().getPath(), file.getPath() );
Review Comment:
```suggestion
file = new File( fileSet.getBasedir(), file.getPath() );
```
##########
maven-scm-providers/maven-scm-providers-git/maven-scm-provider-jgit/src/main/java/org/apache/maven/scm/provider/git/jgit/command/JGitUtils.java:
##########
@@ -527,4 +529,61 @@ public static List<String> getTags( Repository repo, RevCommit commit ) throws I
}
return result;
}
+
+ /**
+ * Remove all files in the given fileSet to the repository.
+ *
+ * @param git the repo to remove the files from
+ * @param fileSet the set of files within the workspace, the files are removed
+ * relative to the basedir of this fileset
+ * @return a list of removed files
+ * @throws GitAPIException
+ * @throws NoFilepatternException
+ */
+ public static List<ScmFile> removeAllFiles( Git git, ScmFileSet fileSet )
+ throws GitAPIException, NoFilepatternException
+ {
+ URI baseUri = fileSet.getBasedir().toURI();
+ RmCommand remove = git.rm();
+ for ( File file : fileSet.getFileList() )
+ {
+ if ( !file.isAbsolute() )
+ {
+ file = new File( fileSet.getBasedir().getPath(), file.getPath() );
+ }
+
+ if ( file.exists() )
+ {
+ URI workingCopyRootUri = git.getRepository().getWorkTree().toURI();
+ String path = FilenameUtils.normalizeFilename( relativize( workingCopyRootUri, file ) );
+ remove.addFilepattern( path );
+ }
+ }
+ remove.call();
+
+ Status status = git.status().call();
+
+ Set<String> allInIndex = new HashSet<String>();
+ allInIndex.addAll( status.getRemoved() );
+ List<ScmFile> removedFiles = new ArrayList<ScmFile>( allInIndex.size() );
+
+ // rewrite all detected files to now have status 'checked_in'
+ for ( String entry : allInIndex )
+ {
+ ScmFile scmFile = new ScmFile( entry, ScmFileStatus.DELETED );
+
+ // if a specific fileSet is given, we have to check if the file is
+ // really tracked
+ for ( Iterator<File> itfl = fileSet.getFileList().iterator(); itfl.hasNext(); )
+ {
+ String path = FilenameUtils.normalizeFilename( relativize( baseUri, itfl.next() ) );
+ if ( path.equals( FilenameUtils.normalizeFilename( scmFile.getPath() ) ) )
Review Comment:
this logic is IMHO incorrect, as `scmFile` has repository relative path names (already with forward slashes) while `path` is relative to basedir.
##########
maven-scm-providers/maven-scm-providers-git/maven-scm-provider-gittest/src/main/java/org/apache/maven/scm/provider/git/command/remove/GitRemoveCommandTckTest.java:
##########
@@ -0,0 +1,85 @@
+package org.apache.maven.scm.provider.git.command.remove;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.io.File;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.StandardOpenOption;
+import java.util.Arrays;
+
+import org.apache.maven.scm.ScmFileSet;
+import org.apache.maven.scm.ScmTckTestCase;
+import org.apache.maven.scm.command.remove.RemoveScmResult;
+import org.apache.maven.scm.provider.git.GitScmTestUtils;
+
+/**
+ * This test tests the remove command.
+ *
+ * @author Georg Tsakumagos
+ */
+public abstract class GitRemoveCommandTckTest
+ extends ScmTckTestCase
+{
+ /** {@inheritDoc} */
+ public void initRepo()
+ throws Exception
+ {
+ GitScmTestUtils.initRepo( "src/test/resources/repository/", getRepositoryRoot(), getWorkingDirectory() );
+ }
+
+ public void testCommandRemoveWithFile()
+ throws Exception
+ {
+ File toBeRemoved = new File( getWorkingDirectory().getAbsolutePath() + File.separator + "toto.xml" );
+
+ Files.write( toBeRemoved.getAbsoluteFile().toPath(),
+ "data".getBytes( StandardCharsets.UTF_8 ), StandardOpenOption.APPEND, StandardOpenOption.CREATE );
+
+ ScmFileSet fileSet = new ScmFileSet( getWorkingCopy(), toBeRemoved );
+ RemoveScmResult removeResult = getScmManager().remove( getScmRepository(), fileSet, getBasedir() );
+ assertResultIsSuccess( removeResult );
+ }
+
+ public void testCommandRemoveWithDirectory()
+ throws Exception
+ {
+ File toBeRemoved = new File( getWorkingDirectory().getAbsolutePath() + File.separator + "toto" );
+ toBeRemoved.mkdir();
+
+ ScmFileSet fileSet = new ScmFileSet( getWorkingCopy(), toBeRemoved );
+ RemoveScmResult removeResult = getScmManager().remove( getScmRepository(), fileSet, getBasedir() );
+ assertResultIsSuccess( removeResult );
+ }
+
+ public void testCommandRemoveWithTwoDirectories()
+ throws Exception
+ {
+ File toBeRemoved1 = new File( getWorkingDirectory().getAbsolutePath() + File.separator + "toto" );
+ toBeRemoved1.mkdir();
+
+ File toBeRemoved2 = new File( getWorkingDirectory().getAbsolutePath() + File.separator + "tata" );
+ toBeRemoved2.mkdir();
+
+ ScmFileSet fileSet = new ScmFileSet( getWorkingCopy(), Arrays.asList( toBeRemoved1, toBeRemoved2 ) );
+ RemoveScmResult removeResult = getScmManager().remove( getScmRepository(), fileSet, getBasedir() );
+ assertResultIsSuccess( removeResult );
+ }
Review Comment:
we should add a test for a basedir != repository root
##########
maven-scm-providers/maven-scm-providers-git/maven-scm-provider-gittest/src/main/java/org/apache/maven/scm/provider/git/command/remove/GitRemoveCommandTckTest.java:
##########
@@ -0,0 +1,85 @@
+package org.apache.maven.scm.provider.git.command.remove;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.io.File;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.StandardOpenOption;
+import java.util.Arrays;
+
+import org.apache.maven.scm.ScmFileSet;
+import org.apache.maven.scm.ScmTckTestCase;
+import org.apache.maven.scm.command.remove.RemoveScmResult;
+import org.apache.maven.scm.provider.git.GitScmTestUtils;
+
+/**
+ * This test tests the remove command.
+ *
+ * @author Georg Tsakumagos
+ */
+public abstract class GitRemoveCommandTckTest
+ extends ScmTckTestCase
+{
+ /** {@inheritDoc} */
+ public void initRepo()
+ throws Exception
+ {
+ GitScmTestUtils.initRepo( "src/test/resources/repository/", getRepositoryRoot(), getWorkingDirectory() );
+ }
+
+ public void testCommandRemoveWithFile()
+ throws Exception
+ {
+ File toBeRemoved = new File( getWorkingDirectory().getAbsolutePath() + File.separator + "toto.xml" );
+
+ Files.write( toBeRemoved.getAbsoluteFile().toPath(),
+ "data".getBytes( StandardCharsets.UTF_8 ), StandardOpenOption.APPEND, StandardOpenOption.CREATE );
+
+ ScmFileSet fileSet = new ScmFileSet( getWorkingCopy(), toBeRemoved );
+ RemoveScmResult removeResult = getScmManager().remove( getScmRepository(), fileSet, getBasedir() );
+ assertResultIsSuccess( removeResult );
Review Comment:
I think we need a consecutive checkout to be sure that `toto.xml` is really removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] kwin commented on pull request #144: [SCM-925] Implement RemoveCommand in maven-scm-provider-jgit
Posted by GitBox <gi...@apache.org>.
kwin commented on PR #144:
URL: https://github.com/apache/maven-scm/pull/144#issuecomment-1192765636
I may find some time next week to look into that. If you want to release M2 earlier just defer this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] michael-o commented on pull request #144: [SCM-925] Implement RemoveCommand in maven-scm-provider-jgit
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #144:
URL: https://github.com/apache/maven-scm/pull/144#issuecomment-1192763757
@kwin Should we stall this for M2 until we get a clearer picture and @G-Ork can chime in?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] michael-o commented on pull request #144: [SCM-925] Implement RemoveCommand in maven-scm-provider-jgit
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #144:
URL: https://github.com/apache/maven-scm/pull/144#issuecomment-1134202218
Maven 1? I was talking about Maven SCM 1.x.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org