Merge lp://qastaging/~amanica/bzr-externals/ecmd_relpath_variable into lp://qastaging/bzr-externals

Proposed by Marius Kruger
Status: Merged
Merged at revision: not available
Proposed branch: lp://qastaging/~amanica/bzr-externals/ecmd_relpath_variable
Merge into: lp://qastaging/bzr-externals
Diff against target: 32 lines (+8/-0)
1 file modified
commands.py (+8/-0)
To merge this branch: bzr merge lp://qastaging/~amanica/bzr-externals/ecmd_relpath_variable
Reviewer Review Type Date Requested Status
Eugene Tarasenko Approve
Review via email: mp+23310@code.qastaging.launchpad.net

Description of the change

Support a {RELPATH} variable in external commands

it makes ecmd extremely powerful to do other missing operations eg:

bzr ecmd -- push --no-strict /tmp/externals_test/{RELPATH}
bzr ecmd -- push --no-strict /tmp/externals_test2/{RELPATH}
bzr ecmd -- bind /tmp/externals_test/{RELPATH}
bzr ecmd -- switch /tmp/externals_test2/{RELPATH}
bzr ecmd -- missing /tmp/externals_test/{RELPATH}

To post a comment you must log in.
Revision history for this message
Alexander Belchenko (bialix) wrote :

Marius Kruger пишет:
> Marius Kruger has proposed merging lp:~amanica/bzr-externals/ecmd_relpath_variable into lp:bzr-externals.
>
> Requested reviews:
> Eugene Tarasenko (eugene-tarasenko)
>
>
> Support a {RELPATH} variable in external commands

I would recommend to use lowercase {relpath} or even {path} instead. I
think using UPPERCASE in scmproj was not good idea.

Revision history for this message
Alexander Belchenko (bialix) wrote :

+ def _substitute_in_commandlist(self, command_list, rel_path):
+ return [x.replace('{RELPATH}', rel_path) for x in command_list]
+

You can use builtin python template syntax here IMO, see
http://docs.python.org/library/string.html?highlight=template#string.Template

Revision history for this message
Marius Kruger (amanica) wrote :

On 13 April 2010 13:27, Alexander Belchenko <email address hidden> wrote:
> +    def _substitute_in_commandlist(self, command_list, rel_path):
> +        return [x.replace('{RELPATH}', rel_path) for x in command_list]
> +
>
> You can use builtin python template syntax here IMO, see
> http://docs.python.org/library/string.html?highlight=template#string.Template

thanks for the feedback Alexander! (the idea for this obviously came
from scmproj)

I agree lowercase would be better.

I tried the templating now, but unfortunately on linux shell, this is
not so nice:
bzr ecmd -- switch /tmp/externals_test2/$relpath #does not do what
one expects, because $ must be escaped:
bzr ecmd -- switch /tmp/externals_test2/\$relpath

I suppose I can live with having to do \$ on linux, but the case where
you forget, it can really break things.

So Eugene what do you think {} or \$ ?

--
<>< Marius ><>

Revision history for this message
Alexander Belchenko (bialix) wrote :

Marius Kruger пишет:
> On 13 April 2010 13:27, Alexander Belchenko <email address hidden> wrote:
>> + def _substitute_in_commandlist(self, command_list, rel_path):
>> + return [x.replace('{RELPATH}', rel_path) for x in command_list]
>> +
>>
>> You can use builtin python template syntax here IMO, see
>> http://docs.python.org/library/string.html?highlight=template#string.Template
>
> thanks for the feedback Alexander! (the idea for this obviously came
> from scmproj)
>
> I agree lowercase would be better.
>
> I tried the templating now, but unfortunately on linux shell, this is
> not so nice:
> bzr ecmd -- switch /tmp/externals_test2/$relpath #does not do what
> one expects, because $ must be escaped:
> bzr ecmd -- switch /tmp/externals_test2/\$relpath
>
> I suppose I can live with having to do \$ on linux, but the case where
> you forget, it can really break things.
>
> So Eugene what do you think {} or \$ ?

Ummm, IIRC string.Template accept and bare {foo} syntax too. Or maybe I
have false memory... Anyway, I don't think using bare $ is good if it
makes the life of Linux users much harder. I don't remember which syntax
git uses.

Revision history for this message
Marius Kruger (amanica) wrote :

On 13 April 2010 13:54, Alexander Belchenko <email address hidden> wrote:
> Ummm, IIRC string.Template accept and bare {foo} syntax too.

it does not

Revision history for this message
Alexander Belchenko (bialix) wrote :

Marius Kruger пишет:
> On 13 April 2010 13:54, Alexander Belchenko <email address hidden> wrote:
>> Ummm, IIRC string.Template accept and bare {foo} syntax too.
>
> it does not

I was wrong then. Please, ignore my suggestion about string.Template.

Revision history for this message
Eugene Tarasenko (etarasenko) wrote :

> So Eugene what do you think {} or \$ ?

The command

  bzr push --no-strict

will be the most correct variant ;) I already intercept some commands and I can try to assort arguments for a command push.

I think {relpath} better.

Revision history for this message
Alexander Belchenko (bialix) wrote :

Eugene Tarasenko пишет:
>> So Eugene what do you think {} or \$ ?
>
> The command
>
> bzr push --no-strict
>
> will be the most correct variant ;) I already intercept some commands and I can try to assort arguments for a command push.

I think you missed the point.

bzr ecmd -- push --no-strict /tmp/externals_test/{RELPATH}

will push entire(?) project to /tmp/externals_test/

>
> I think {relpath} better.

Revision history for this message
Eugene Tarasenko (etarasenko) wrote :

Then can be better to make so?

bzr push --no-strict /tmp/externals_test

If it is not feature branch, plugin must correct work with feature branch.

Revision history for this message
Alexander Belchenko (bialix) wrote :

Eugene Tarasenko пишет:
> Then can be better to make so?
>
> bzr push --no-strict /tmp/externals_test
>
> If it is not feature branch, plugin must correct work with feature branch.
>

How bzr-externals will differentiate between 2 cases:

1) I want to push my root branch to specified URL and all components to
their locations specified in .bzrmeta/externals config

and

2) I want to mirror entire project into other local directory

?

In both cases "push" command is involved.

Revision history for this message
Eugene Tarasenko (etarasenko) wrote :

Sorry, the letter is not has reached.

In the second variant you want that the main project was copied only or not? I don't understand. If You need push full copy:

1) bzr push bzr://server/proj
2) bzr push ../local_push

There is a plugin already so works.

Revision history for this message
Eugene Tarasenko (etarasenko) wrote :

Thanks!

review: Approve
42. By Eugene Tarasenko

Support a {relpath} variable in ecmd command

Revision history for this message
Alexander Belchenko (bialix) wrote :

Eugene Tarasenko пишет:
> Sorry, the letter is not has reached.
>
> In the second variant you want that the main project was copied only or not? I don't understand. If You need push full copy:
>
> 1) bzr push bzr://server/proj
> 2) bzr push ../local_push
>
> There is a plugin already so works.

Тяжело с тобой общаться на английском.

Вариант 1) Я хочу сделать push главной ветки (куда-то на сервер) и всех
компонентов по тем URL, которые указаны в .bzrmeta/externals

2) Я хочу сделать копию проекта на локальном диске

Во втором случае push не применим. Если ты будуешь делать разное
поведение команды push в зависимости от локального пути/URL к удаленному
серверу, это будет неверно.

Revision history for this message
Eugene Tarasenko (etarasenko) wrote :

Да, английский мой враг ;(

> Я хочу сделать копию проекта на локальном диске

А это правильно делать копию командой push, а не branch?
Даже если так делать можно, то плагин умеет так работать уже сейчас, он для каждой ветки проверяет ее наличие в target_dir на локальном компьютере и в зависимости от этого делает или push, или branch. На этом принципе организована работа с feature branch.

Revision history for this message
Alexander Belchenko (bialix) wrote :

Eugene Tarasenko пишет:
> Да, английский мой враг ;(
>
>> Я хочу сделать копию проекта на локальном диске
>
> А это правильно делать копию командой push, а не branch?

Не знаю.

> Даже если так делать можно, то плагин умеет так работать уже сейчас, он для каждой ветки проверяет ее наличие в target_dir на локальном компьютере и в зависимости от этого делает или push, или branch. На этом принципе организована работа с feature branch.

Не понял.

Revision history for this message
Eugene Tarasenko (etarasenko) wrote :

Прошу прощения, я ввел в заблуждение - алгоритм проверки существования директории используется для команды pull, в зависимости от этого может вызваться либо команда branch, либо pull для внешней ветки.

Я думал что для команды push используется похожий алгоритм, но оказалось что нет - здесь вызывается post_change_branch_tip_hook если таргет директории не существует, либо post_push_hook если существует. Даже не знаю баг это или фича, но работает как надо ;)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches