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

Use XML output to compute SVN.is_pristine() #3653

Merged
merged 17 commits into from Oct 29, 2018

Conversation

@jgsogo
Copy link
Member

@jgsogo jgsogo commented Oct 1, 2018

Changelog: Fix: Use XML output from SVN command line interface to compute if the repository is pristine.

  • closes #3640
  • wait for #3192 before merging this one!

Tests needed (from comments):

  • An svn:externals definition without fixed revision --> should lead to error, because not fixed
  • An svn:externalsdefinition with pegged revision
  • An svn:externalsdefinition with pegged revision & operative revision --> could be different revsisions, we must use operative revision
  • Also a test, where the working copy is not able to reach the repository (Just delete the local repository) would be nice. Then it would be nice, if we where still able to run is_pristine function.
  • Also it is possible to make a single file link instead of a directory. I never used this, but may be we should test it?
@jgsogo jgsogo added this to the 1.9 milestone Oct 1, 2018
@ghost ghost assigned jgsogo Oct 1, 2018
@ghost ghost added the stage: review label Oct 1, 2018
conans/client/tools/scm.py Outdated Show resolved Hide resolved
@jgsogo
Copy link
Member Author

@jgsogo jgsogo commented Oct 9, 2018

Note.- If externals do not define a revision the is_pristine implementation will fail... but pristine with open revisions does not make any sense 🤔

@memsharded memsharded assigned memsharded and unassigned jgsogo Oct 9, 2018
class SVNToolTestsPristineWithExternals(SVNLocalRepoTestCase):

def _propset_cmd(self, relpath, rev, url):
return 'propset svn:externals "{} -r{} {}" .'.format(relpath, rev, url)
Copy link
Contributor

@climblinne climblinne Oct 11, 2018

I thought the definition is `svn:externals "-r148 http://svn.example.com/skinproj third-party/skins", so ' relpath' and 'revsion' changed

Copy link
Contributor

@climblinne climblinne Oct 11, 2018

I just recognized, that this is the new scheme (from Subversion 1.5). The old is still usable.

Copy link
Contributor

@climblinne climblinne Oct 11, 2018

These means, we have to check both schemes, because we have to find the revision & local path of it. We have to watch, because there are also some different whitepace possibilities:

$ svn propget svn:externals paint
http://svn.thirdparty.com/repos/My%20Project "My Project"
http://svn.thirdparty.com/repos/%22Quotes%20Too%22 \"Quotes\ Too\"
$

Copy link
Member Author

@jgsogo jgsogo Oct 22, 2018

But Conan works only from the consumer point of view, and I hope --xml output will be the same regardless of the externals property syntax.

conans/client/tools/scm.py Outdated Show resolved Hide resolved
conans/client/tools/scm.py Outdated Show resolved Hide resolved
@jgsogo
Copy link
Member Author

@jgsogo jgsogo commented Oct 15, 2018

I've added some tests. I'm finding it quite difficult to build tests using a peg_revision 😖

@lasote
Copy link
Contributor

@lasote lasote commented Oct 19, 2018

Conflicts


shutil.rmtree(repo_url, ignore_errors=False, onerror=try_remove_readonly)
self.assertFalse(os.path.exists(repo_url))
with self.assertRaisesRegexp(subprocess.CalledProcessError, "non-zero exit status 1"):
Copy link
Member

@memsharded memsharded Oct 29, 2018

This shouldn't be an Exception, should return False, to allow creating the package, even if it is not pristine.

@memsharded memsharded merged commit 0816e3c into conan-io:develop Oct 29, 2018
2 checks passed
@ghost ghost removed the stage: review label Oct 29, 2018
@jgsogo jgsogo deleted the issue/3640-svn-is-pristine branch Oct 29, 2018
grisumbras pushed a commit to grisumbras/conan that referenced this issue Dec 27, 2018
Use XML output to compute SVN.is_pristine()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants