Bug 16075 - [FR] gear-update-tag: add "verify" mode
: [FR] gear-update-tag: add "verify" mode
Status: CLOSED FIXED
: Sisyphus
(All bugs in Sisyphus/gear)
: unstable
: all Linux
: P2 enhancement
Assigned To:
:
: http://git.altlinux.org/people/raorn/...
: patch
:
:
  Show dependency tree
 
Reported: 2008-06-17 21:05 by
Modified: 2008-08-24 01:41 (History)


Attachments


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2008-06-17 21:05:53
Когда в .gear/rules используются левые бранчи, которые мержатся в master, очень
легко забыть запустить gear-update-tag перед сборкой пакета.  Хочется иметь
опцию --verify, которая проверяла, все ли теги в .gear/tags смотрят куда
положено.  Ну и выходила бы с ненулевым статусом если нет.
------- Comment #1 From 2008-06-18 03:29:40 -------
Параметр --verify для gear-update-tag не помешает.
Только ведь можно и --verify забыть запустить.
Поэтому мне кажется логичным научить "gear-update-tag --verify" работать с
commitish и добавить соответствующий вызов в gear-create-tag.
------- Comment #2 From 2008-06-18 15:41:27 -------
Я приделаю commitish к gear-update-tag.

Не уверен насчёт gear-create-tag.  Логичней было бы gear в режиме
--rpm/--hasher/pkg.tar.
------- Comment #3 From 2008-06-18 21:34:46 -------
Сделал, но если в .gear/tags используются имена бранчей а не тегов, толку от
этого будет мало.
------- Comment #4 From 2008-06-18 21:36:31 -------
Reassing и жду замечаний или мержа.
------- Comment #5 From 2008-06-19 01:02:48 -------
(In reply to comment #3)
> Сделал, но если в .gear/tags используются имена бранчей а не тегов, толку от этого
> будет мало.
> 

check_tag_name() не позволит использовать не тэги. Собственно в этом и проблема
#15610.
------- Comment #6 From 2008-06-19 01:29:13 -------
(In reply to comment #5)
> check_tag_name() не позволит использовать не тэги.

И давно?  См. напр. mutt1.5.
------- Comment #7 From 2008-06-19 02:10:40 -------
(In reply to comment #6)
> И давно?  См. напр. mutt1.5.

Интересно:

check_tag_name() 
{ 
  local name="$1" && shift 

  [ -n "$name" ] || 
    rules_error 'Empty tag name is not allowed' 
  [ -n "${name##/*}" ] || 
    rules_error "Invalid tag name \"$name\": initial '/' is not allowed" 
  [ -n "${name%%*/}" ] || 
    rules_error "Invalid tag name \"$name\": trailing '/' is not allowed" 
  [ -n "${name##*[][*?^~:@[:space:]]*}" ] || 
    rules_error "Invalid tag name \"$name\": invalid characters found" 
  git check-ref-format "tags/$name" || 
    rules_error "Invalid tag name \"$name\"" 
}

это функция проверки имени тэга. Начальные проверки имени ясны. Интересен git
check-ref-format.

[legion gear]$ git check-ref-format 'tags/foobar'; echo $?
0
[legion gear]$ git rev-parse 'tags/1.0.1-alt1'; echo $?
0a47c66dedd606aeb028f6c8db65ab447c6e211a
0
[legion gear]$ git check-ref-format 'tags/foobar'; echo $?
0
[legion gear]$ git rev-parse 'tags/foobar'; echo $?
tags/foobar
fatal: ambiguous argument 'tags/foobar': unknown revision or path not in the
working tree.
Use '--' to separate paths from revisions
128

Кажется поэтому ты и можешь использовать имена бранчей.

Либо я в чём-то сильно ошибаюсь.
------- Comment #8 From 2008-06-19 02:17:50 -------
Хотя нужно внимательнее прочитать resolve_commit_name().
------- Comment #9 From 2008-06-19 02:22:24 -------
Хм.

$ git check-ref-format tags/master && echo OK || echo ERR
OK
$ git check-ref-format master && echo OK || echo ERR 
ERR
$ git check-ref-format refs/heads/master && echo OK || echo ERR 
OK
$ git check-ref-format refs/tags/master && echo OK || echo ERR 
OK

Хм. Хм.

$ git show-ref master
a601b50cc564a73ddcd777c76cfe45a44317cc58 refs/heads/master
a601b50cc564a73ddcd777c76cfe45a44317cc58 refs/remotes/origin/master
$ git show-ref tags/master
[1]    18797 exit 1     git show-ref tags/master
$ git show-ref refs/heads/master
a601b50cc564a73ddcd777c76cfe45a44317cc58 refs/heads/master
$ git show-ref refs/tags/master 
[1]    18847 exit 1     git show-ref refs/tags/master
------- Comment #10 From 2008-06-19 02:27:19 -------
(In reply to comment #9)
> Хм.
> 
> $ git check-ref-format tags/master && echo OK || echo ERR

Вот-вот ... такое ощущение, что раньше check-ref-format проверял не только
корректность имени, но и существование этого объекта.
------- Comment #11 From 2008-06-19 02:33:41 -------
ну и разумеется:

gear-update-tag "name" -> check_tag_name_from_command_line -> check_tag_name

поэтому бранчи вместо тэгов можно создавать.
------- Comment #12 From 2008-06-19 02:59:59 -------
Мне бы хотелось чтобы это так и осталось...  Иначе придётся лепить tip'ы на
каждый бранч, что ещё неудобнее.
------- Comment #13 From 2008-06-19 13:13:43 -------
Тут получается сложная ситуация ... бранч нарушает концепцию однозначной
определённости коммита. Но такая конструкция уже используется и ты этому
иллюстрация.

Вообщем, нужно думать как это решать.

P.S. Мне кажется, что всё-таки нужно запрещать использование бранчей. Это
ломает всю идею этой утилиты.
------- Comment #14 From 2008-06-19 13:34:16 -------
Как раз наоборот.  Эта утилита фиксирует имена тегов/бранчей на момент сборки. 
Мне удобно использовать бранчи, зачем усложнять людям жизнь?  Выстрелить себе в
ногу можно и тегами вида BRANCHNAME-tip, только тут ещё больше шансов
накосячить так, что никто этого не заметит (уже чинил такое).
------- Comment #15 From 2008-06-19 13:39:03 -------
(In reply to comment #14)
> Как раз наоборот.  Эта утилита фиксирует имена тегов/бранчей на момент
> сборки.  Мне удобно использовать бранчи, зачем усложнять людям жизнь? 
> Выстрелить себе в ногу можно и тегами вида BRANCHNAME-tip, только тут ещё больше
> шансов накосячить так, что никто этого не заметит (уже чинил такое).
> 

Я тоже иногда использую бранчи, см. coreutils.git
------- Comment #16 From 2008-06-19 13:58:56 -------
Угу. gear-update-tag фиксирует sha1 объекта при добавлении в .gear/tags.

Тогда Лёшь, как быть с 16075#c3 ?

P.S. Слова "мне так удобно" или "я так привык" за агрумент не считаю.
------- Comment #17 From 2008-06-19 14:07:17 -------
Для меня загадка почему vsu так ограничил "diff: ...". К сожалению он на письма
тоже не отвечает.
------- Comment #18 From 2008-06-19 14:20:27 -------
(In reply to comment #16)
> Тогда Лёшь, как быть с 16075#c3 ?
Симриться.  Тега, который был зафиксирован при помощи gear-update-tag может не
быть в репозитарии или он уже может указывать на другой об'ект.  Именно для
этого и делался gear-update-tag.

gear-update-tag работает с working copy, --verify только проверяет что запуск
gear-update-tag --all в данный момент ничего не сделает.

> P.S. Слова "мне так удобно" или "я так привык" за агрумент не считаю.
Как насчёт слов "неувеличение энтропии"?  Зачем делать два и более действий
там, где достаточно одного?
------- Comment #19 From 2008-06-19 14:35:35 -------
(In reply to comment #18)
> (In reply to comment #16)
> > Тогда Лёшь, как быть с 16075#c3 ?
> Симриться.  Тега, который был зафиксирован при помощи gear-update-tag может не быть
> в репозитарии или он уже может указывать на другой об'ект.  Именно для этого
> и делался gear-update-tag.

Как я понимаю сейчас gear-update-tag поступает с бранчами также как тэгами. Он
и те и другие имена заносит в .gear/tags и вычитывает из rules т.е. по коду они
не отличимы.

Почему же ты считаешь что при использовании бранчей от --verify толку будет
мало?

> gear-update-tag работает с working copy, --verify только проверяет что запуск gear-update-tag --all в
> данный момент ничего не сделает.

Нам нужно исправлять gear-update-tag для корректной работы с бранчами. Сейчас
для бранчей он не полнофункционален. Вот пример:

if [ -n "$tag_name" ] && [ -z "$delete" ]; then
  if [ -z "$tag_value" ]; then
    tag_value="$(get_object_sha1 refs/tags/"$tag_name")" ||
      fatal "Invalid tag \"$tag_name\""
...

Нужно думать как убрать неоднозначность $tag_name.
------- Comment #20 From 2008-06-19 14:48:01 -------
Затеял исправление этой утилиты под наши реали:

http://git.altlinux.org/people/legion/packages/gear.git?p=gear.git;a=shortlog;h=refs/heads/gear-update-tag
------- Comment #21 From 2008-06-19 14:49:07 -------
(In reply to comment #19)
> Почему же ты считаешь что при использовании бранчей от --verify толку будет
> мало?

Я не так выразился.  Мало толку будет от использования --verify --tree-ish в
том
случае когда об'екты (бранчи или теги) из .gear/tags изменились.  В бранч
накоммитили или tip-тег подвинули - без разницы.

Я не знаю как 100% определить случай, когда человек забыл подвинуть tip-тег,
поэтому использую имена бранчей, кроме обязательного требования
использовать только верионные tip-теги, но это уже помойму перебор.
------- Comment #22 From 2008-06-19 14:58:57 -------
(In reply to comment #21)
> Я не так выразился.  Мало толку будет от использования --verify --tree-ish в том
> случае когда об'екты (бранчи или теги) из .gear/tags изменились.  В бранч
> накоммитили или tip-тег подвинули - без разницы.

А почему нельзя проверить sha1 из .gear/tags c sha1 на который указывает
тег/бранч ?
------- Comment #23 From 2008-06-19 15:03:52 -------
(In reply to comment #22)
> А почему нельзя проверить sha1 из .gear/tags c sha1 на который указывает тег/бранч ?
Не понял вопроса.
------- Comment #24 From 2008-06-19 16:04:38 -------
> Я не знаю как 100% определить случай, когда человек забыл подвинуть tip-тег,
> поэтому использую имена бранчей, кроме обязательного требования
> использовать только верионные tip-теги, но это уже помойму перебор.

Так. Чтобы не толочь воду в ступе, можешь более детально поясинть, какую
ситуацию нужно определить ? 
------- Comment #25 From 2008-06-19 16:52:36 -------
.gear/rules:
tar: master:.

Если забыть обновить .gear/tags при сборке нового пакета соберётся предыдущая
версия с новым спеком.  gear не ругнётся, поскольку HEAD растёт из "старого
master".
------- Comment #26 From 2008-06-19 16:59:32 -------
(In reply to comment #25)
> .gear/rules:
> tar: master:.
> 
> Если забыть обновить .gear/tags при сборке нового пакета соберётся предыдущая
> версия с новым спеком.  gear не ругнётся, поскольку HEAD растёт из "старого master".
> 

Т.е. если пользователь передвинул тег перечисленный в .gear/tags и не сказал
gear-update-tag ?
------- Comment #27 From 2008-06-19 17:09:13 -------
Именно.
------- Comment #28 From 2008-06-19 17:19:15 -------
(In reply to comment #26)
> Т.е. если пользователь передвинул тег перечисленный в .gear/tags и не сказал
> gear-update-tag ?

Получается что в .gear/tags сохранено старое (предыдущее) состояние. Оно
выглядет например так:

$ cat .gear/tags/list 
d138e5e3a47f7fced4435116e3cd9210b3e0d1fc aterm-1.0.1

Нам нужно посмотреть на какой объект указывает aterm-1.0.1 сейчас и сравнить
его с d138e5e3a47f7fced4435116e3cd9210b3e0d1fc (старым значением). Если они не
совпадут, то совершенно понятно, что тег aterm-1.0.1 передвинули, но не
обновлили .gear/tags/list. Разве нет ?
------- Comment #29 From 2008-06-19 17:34:19 -------
Ты издеваешься, да?  Именно это и делает gear-update-tag --verify.
------- Comment #30 From 2008-06-19 17:40:25 -------
(In reply to comment #29)
> Ты издеваешься, да?  Именно это и делает gear-update-tag --verify.

Именно. Я к тому что я не очень понимаю проблемы 16075#c25. Разве что
отсутствия такой же проверки в gear.
------- Comment #31 From 2008-06-19 17:47:09 -------
Ты всё-таки надо мной издеваешься.  Зачем ты это делаешь?
------- Comment #32 From 2008-06-19 17:55:43 -------
(In reply to comment #31)
> Ты всё-таки надо мной издеваешься.  Зачем ты это делаешь?

Я вижу мы с тобой никак не можем найти общий язык. Я этот флейм прекращаю. Твои
изменения я смерджил в gear-next. Дима их может взять либо у меня, либо у тебя.

Сс: -legion@
------- Comment #33 From 2008-06-23 15:22:08 -------
(In reply to comment #15)
> (In reply to comment #14)
> > Как раз наоборот.  Эта утилита фиксирует имена тегов/бранчей на момент
> > сборки.  Мне удобно использовать бранчи, зачем усложнять людям жизнь? 
> > Выстрелить себе в ногу можно и тегами вида BRANCHNAME-tip, только тут ещё больше
> > шансов накосячить так, что никто этого не заметит (уже чинил такое).
> 
> Я тоже иногда использую бранчи, см. coreutils.git

Погорячился. Не использую я бранчи в .gear/rules
------- Comment #34 From 2008-06-23 16:04:05 -------
(In reply to comment #33)
> Погорячился. Не использую я бранчи в .gear/rules 

А это не важно. Если есть хоть один человек, то нет гарантии что он такой один.
Я даже не знаю как долго у нас разрешено такое поведение.
------- Comment #35 From 2008-07-17 20:43:45 -------
По поводу разрешения использования бранчей вместо тегов, как сделано в
http://git.altlinux.org/people/legion/packages/?p=gear.git;a=commitdiff;h=376cbf3c9fe77c78eb4f61872db16cd0b85be7c3
- уже не помню, почему я написал именно так, но в принципе в этом месте можно
использовать даже git rev-parse --verify, поскольку этот код в любом случае
выполняется у мантейнера. В случае неоднозначности всегда можно вызвать
gear-update-tag с двумя параметрами (точно так же можно засунуть туда бранч
вместо тега уже сейчас).

Вызов git check-ref-format был предназначен только для отлова недопустимых
символов, причём "tags/" там используется только из-за того, что git
check-ref-format требует наличия хотя бы одного символа "/" в параметре.  На
сам ом деле сейчас это работает не совсем правильно, поскольку текущий код
передаёт в git check-ref-format конструкции вида @name@, рассчитывая на то, что
они пройдут проверку, однако сейчас в git символ '@' имеет специальное
значение, просто git check-ref-format почему-то его не отлавливает.
------- Comment #36 From 2008-08-24 01:39:55 -------
Исправлено. Закрываю.
------- Comment #37 From 2008-08-24 01:41:12 -------
Ключ добавлен.