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

Pass username and password as extra options to SVN commands #5601

Merged
merged 1 commit into from Aug 12, 2019

Conversation

@jasal82
Copy link
Contributor

commented Aug 9, 2019

Changelog: Bugfix: SVN uses username and password if provided
Docs: omit

closes #5599

@tags: svn, slow

@CLAassistant

This comment has been minimized.

Copy link

commented Aug 9, 2019

CLA assistant check
All committers have signed the CLA.

@jgsogo

This comment has been minimized.

Copy link
Member

commented Aug 9, 2019

Hi, @jasal82 , I would like to check how it behaves when the URL uses ssh: svn+ssh://..., does the SVN arguments work?

@jasal82

This comment has been minimized.

Copy link
Contributor Author

commented Aug 9, 2019

My server does not support that protocol so I get a connection error. There was no warning about the arguments, probably they are ignored in that case.

@jgsogo
jgsogo approved these changes Aug 9, 2019
Copy link
Member

left a comment

This adds missing functionality to SVN

@jgsogo

This comment has been minimized.

Copy link
Member

commented Aug 9, 2019

I've changed the first commit of this PR to add the Changelog and Docs keywords, they are used by an automatic tool to collect the changelog.

Copy link
Member

left a comment

Yes, this is ok, adding missing functionality.

My main concern is about the possibility of having those credentials printed in plain text in logs, as the underlying object ConanRunner, has an option to print commands to stdout (and to log them).

Could you please check @jgsogo if there is an easy way to intercept those and hide them from output?
If this is already affecting the Git component, then this can be merged, and please open a new issue to handle it, try to improve this for 1.19. Thanks.

@memsharded memsharded assigned jgsogo and unassigned memsharded Aug 12, 2019
@memsharded memsharded added this to the 1.19 milestone Aug 12, 2019
@jgsogo

This comment has been minimized.

Copy link
Member

commented Aug 12, 2019

For the use case that originates this PR (SCM feature), the possibility will be there as the username and password are in plain text written into the conanfile.py file.

Right commands from Git or SVN are not printed to console, neither with the print_run_commands=True config option. That option only affects the ConanRunner class which is not passed as the runner argument to Git/SVN... I can add a test to check that we don't change this behavior in the future accidentally.

@jgsogo jgsogo merged commit 2dfadb1 into conan-io:develop Aug 12, 2019
2 checks passed
2 checks passed
continuous-integration/jenkins/pr-head This commit looks good
Details
license/cla Contributor License Agreement is signed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.