Do not fail when no scp command is found, unless it is actually needed to fetch something


Submitter:	Oz Linden	Reviewers
Branch:		Groups:	viewer
Bugs:		People:
Change Number:	None	Repository:	autobuild

Description:

During initialization, if there is no scp or pscp command found then autobuild fails immediately. This is true whether or not any scp urls need to be used.

This change modifies the behavior so that a warning is printed if no command is found, but execution proceeds until an scp command is needed, at which time execution fails with an explanatory message.

This patch can print the warning multiple times - I didn't attempt to suppress the extras.

Testing Done:

I've tested this locally, simulating the error by temporarily modifying the names of the commands to be found for scp.

I have not checked it on Windows, where the original problem was found.

Expand All

Looks plausible at first sight.

autobuild/common.py (Diff revision 1)

Why send out a warning at this point?  Developers who will never use scp don't benefit from this message.  If scp is needed and is not present having the error sent out later should be sufficient.

Oz Linden Jan. 28, 2011, 6:34 a.m. (Jan. 28, 2011, 6:34 a.m.)

I confess to being biased in favor of verbosity.  I'd be ok with changing this so that it only comes out if --verbose or --debug are specified (I'd have to figure out how to do that, but that's ok).

Tested on Windows 7 32-bit and works all the needed files for NonStandalone do not DL at least the error is gone.

Worked as designed/explained. For other dummies like me if you did setup.py install before the patch,  you need to repeat it to get the new patched version working in python.

autobuild/common.py (Diff revision 1)

Why not use None here? I think that is more Pythonie.

autobuild/common.py (Diff revision 1)

...and test for None here.

Boroondas Gupte Jan. 29, 2011, 3:40 a.m. (Jan. 29, 2011, 3:40 a.m.)

Good idea. This would avoid bugs that could occur e.g. when the string is changed and not spelled the same in both places.

Diff
- added Diff r2

Description:

removed the (possibly repeated) warnings about not finding an scp command
changed to using None properly rather than a fixed string value

(added logging that should have shown for doing scp copies, but --verbose does not seem to do anything.... different issue)

this is crashing devenv when you do autobuild build -c OpenSourceRelWithDebInfo  on a windows system

Wolfpup Lowenhar Jan. 29, 2011, 7:45 a.m. (Jan. 29, 2011, 7:45 a.m.)

When i tested this second dif was not a clean reop will retest again after deleating build tree

Wolfpup Lowenhar Jan. 30, 2011, 5:23 a.m. (Jan. 30, 2011, 5:23 a.m.)

tested new diff with fresh build tree and it dose work on windows 7.

autobuild/common.py (Diff revisions 1 - 2)

The docstring of find_executable should probably indicate that ...

autobuild/common.py (Diff revisions 1 - 2)

... the return value is None when no matching executable can be found, as we (somewhat) rely on what the return value is in that case. (Not your fault, the old code already relied on that, too.)

autobuild/common.py (Diff revisions 1 - 2)

Usually one would test for None with
        if self._scp is None:
as testing for `not self._scp` will also trigger when the value is False (obviously) or (maybe less obvious) becomes False when casted to bool.

However, because we don't want self._scp to be an empty string when building the command below ...

autobuild/common.py (Diff revisions 1 - 2)

... testing for `not self._scp` here might actually be better, I'm not sure.

You have a pending review.

Other reviews

Your comment

Review Board 1.6.11

This change has been marked as submitted.