[ https://issues.apache.org/jira/browse/SCM-925?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17542272#comment-17542272 ]
ASF GitHub Bot commented on SCM-925: ------------------------------------ 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? > RemoveCommand unsupported - maven-release-plugin:prepare-with-pom failing > ------------------------------------------------------------------------- > > Key: SCM-925 > URL: https://issues.apache.org/jira/browse/SCM-925 > Project: Maven SCM > Issue Type: Bug > Components: maven-scm-provider-jgit > Affects Versions: 1.11.2 > Reporter: Georg Tsakumagos > Assignee: Michael Osipov > Priority: Major > Attachments: maven-scm-provider-jgit.log > > Time Spent: 10m > Remaining Estimate: 0h > > The git scm plugins differs in support for the scm command _remove_. This > breaks the goal _prepare-with-pom_ of the _maven-release-plugin_ using the > _maven-scm-provider-jgit_. The method is not fully implemented and throws an > UnsupportedOperationException. > Referring to the ["Maven SCM Providers > Matrix"|https://maven.apache.org/scm/matrix.html] this feature is documented > as supported and should be implemented in the _jgit_ provider like in > maven-scm-provider-gitexe. > > ||Variant||remove supported||Source|| > |maven-scm-provider-gitexe|(/)|[Github - GitExeScmProvider.java - > getRemoveCommand()|https://github.com/apache/maven-scm/blob/a1f7e9857076940c878a370ff599b394df768d33/maven-scm-providers/maven-scm-providers-git/maven-scm-provider-gitexe/src/main/java/org/apache/maven/scm/provider/git/gitexe/GitExeScmProvider.java#L97]| > |maven-scm-provider-jgit|(x)|[Github - JGitScmProvider.java - > getRemoveCommand()|https://github.com/apache/maven-scm/blob/a1f7e9857076940c878a370ff599b394df768d33/maven-scm-providers/maven-scm-providers-git/maven-scm-provider-jgit/src/main/java/org/apache/maven/scm/provider/git/jgit/JGitScmProvider.java#L113]| > -- This message was sent by Atlassian Jira (v8.20.7#820007)