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

Поддержка различных вариантов сборки #45

Merged
merged 6 commits into from
Jan 16, 2021
Merged

Поддержка различных вариантов сборки #45

merged 6 commits into from
Jan 16, 2021

Conversation

vvd170501
Copy link
Collaborator

No description provided.

@vvd170501
Copy link
Collaborator Author

Таргеты описываются в targets.cfg. Один такой файл создаётся в корне воркспейса (для создания в уже чуществующем нужно вызвать kks init -f). Также можно описывать таргеты для отдельных задач, если создать targets.cfg в рабочей директории.

По сути, получилось что-то вроде сильно упрощённого Makefile.

kks/util/common.py Show resolved Hide resolved
kks/util/config.py Outdated Show resolved Hide resolved
@darkkeks
Copy link
Owner

Кажется, если уже давать изменять сборку, то надо сразу возможность переопределять GCC_ARGS
Сейчас, например, все еще нету возможности убрать оттуда -Werror, если это кому-то мешает

@darkkeks
Copy link
Owner

Я кстати так и не придумал, почему мейкфайл плохо
Какой-нибудь синтаксис учить все равно придется
Зато, с мейкфайлом можно оставить там строку компиляции, и всем будет очевидно как собирается бинарь
Сейчас это спрятано, что я сам понятия не имею как в итоге выглядит строка компиляции, пока не посмотрю в код

@vvd170501
Copy link
Collaborator Author

Кажется, если уже давать изменять сборку, то надо сразу возможность переопределять GCC_ARGS

Можно перенести дефолтные GCC_ARGS в конфиг, но тогда возникает следующая проблема: если вдруг окажется, что какого-то дефолтного флага не хватает (и есть смысл его добавить, например, как было с -lm), то всем пользователям придётся это делать вручную.

@vvd170501
Copy link
Collaborator Author

vvd170501 commented Jan 15, 2021

Насчёт make - здесь есть 2 проблемы:

  1. Это дополнительная зависимость, которую нельзя установить автоматически вместе с пакетом.
  2. Сложнее реализовать (наверное, это возможно, но пока непонятно, как) возможность использования конфигурации по умолчанию, которую можно переопределять для отдельных задач.

@darkkeks
Copy link
Owner

если вдруг окажется, что какого-то дефолтного флага не хватает

Мне кажется правильным по умолчанию не создавать никаких конфигов.
А если человек добавил себе конфиг на воркспейс, то он наверное знает что делает и справится добавить аргумент при необходимости?

То есть для обычного человека все еще можно добавлять конфиги для отдельных задач
А для остальных, будет использоваться обновляемый дефолтный

@vvd170501
Copy link
Collaborator Author

Мне кажется правильным по умолчанию не создавать никаких конфигов.

В таком случае возникает следующая проблема: если пользователь хочет создать таргет, отличающийся от дефолтного несколькими флагами (и конфиг не существует), то ему придётся искать где-то этот самый конфиг (в коде?).

@vvd170501
Copy link
Collaborator Author

Как вариант - можно реализовать что-то подобное (похоже на логику обновления в менеджерах пакетов, например, apt):

  1. При вызове kks run/test конфиг ищется в рабочей директории. Если он найден, считаем, что пользователь знает, что делает, и при необходимости обновит этот конфиг сам (скорее всего, это и не понадобится, т.к. раньше конфиг подходил для этой задачи).

  2. Если для задачи нет локального конфига, ищем дефолтный. Если оказывается, что он устарел и не модифицировался (например, можно хранить хеши старых версий), обновляем его.

  3. Если оказывается, что конфиг был изменён, сохраняем новую версию рядом с ним и предлагаем смерджить их вручную.

Чтобы быстро определять версию конфига, можно просто хранить в нём отдельное поле - всё равно при запуске конфиг читается полностью

@darkkeks
Copy link
Owner

Мне кажется правильным по умолчанию не создавать никаких конфигов.

В таком случае возникает следующая проблема: если пользователь хочет создать таргет, отличающийся от дефолтного несколькими флагами (и конфиг не существует), то ему придётся искать где-то этот самый конфиг (в коде?).

Добавить команду, создающую дефолтный конфиг в текущей папке?

@darkkeks
Copy link
Owner

Как вариант - можно реализовать что-то подобное (похоже на логику обновления в менеджерах пакетов, например, apt):

  1. При вызове kks run/test конфиг ищется в рабочей директории. Если он найден, считаем, что пользователь знает, что делает, и при необходимости обновит этот конфиг сам (скорее всего, это и не понадобится, т.к. раньше конфиг подходил для этой задачи).

  2. Если для задачи нет локального конфига, ищем дефолтный. Если оказывается, что он устарел и не модифицировался (например, можно хранить хеши старых версий), обновляем его.

  3. Если оказывается, что конфиг был изменён, сохраняем новую версию рядом с ним и предлагаем смерджить их вручную.

Чтобы быстро определять версию конфига, можно просто хранить в нём отдельное поле - всё равно при запуске конфиг читается полностью

Если конфиг дефолтный, то зачем его хранить? Я предлагаю

  1. Если есть локальный, используем локальный
  2. Если есть глобальный, используем глобальный. Считаем, что он не дефолтный (иначе его бы не было).
    При создании глобального предупредить, что туда не будут добавлены новые опции при обновлении kks

И глобальный создавать в init какой нибудь опцией

@vvd170501
Copy link
Collaborator Author

Если убрать автоматическое создание конфига, то есть смысл сделать отдельную команду, которая будет его создавать (вместо опции для init). Тогда будет проще создавать локальные конфиги (вместо создания и копирования глобального нужен один вызов команды + не обязательно наличие воркспейса). Предупреждение тогда можно выводить в любом случае -- ни глобальный, ни локальные конфиги не будут обновляться автоматически.

Как в таком случае стоит назвать команду для создания конфига?

@darkkeks
Copy link
Owner

Как в таком случае стоит назвать команду для создания конфига?

Да не важно особо
Я думал опцию для init, которая на самом деле просто создает конфиг, а не делает init, чтобы не плодить команды
Тогда kks init -c или kks init --config выглядит довольно логично

@vvd170501
Copy link
Collaborator Author

Вроде всё готово

@vvd170501 vvd170501 requested a review from darkkeks January 16, 2021 11:07
kks/cmd/init.py Outdated Show resolved Hide resolved
kks/cmd/init.py Show resolved Hide resolved
kks/cmd/init.py Outdated Show resolved Hide resolved
kks/cmd/init.py Outdated Show resolved Hide resolved
kks/cmd/init.py Outdated Show resolved Hide resolved
kks/cmd/test.py Outdated Show resolved Hide resolved
kks/util/config.py Show resolved Hide resolved
kks/binary.py Show resolved Hide resolved
kks/binary.py Outdated Show resolved Hide resolved
kks/util/config.py Outdated Show resolved Hide resolved
@darkkeks
Copy link
Owner

Почему-то на твоей ветке команды заметно дольше работают:

Время работы

Мастер

darkkeks:~/Documents/hse-homework/caos/sm06/4$ time kks top --help

real	0m0.314s
user	0m0.263s
sys	0m0.045s

Твоя ветка

darkkeks:~/Documents/hse-homework/caos/sm06/4$ time kks top --help

real	0m0.536s
user	0m0.456s
sys	0m0.073s

Твоя ветка с патчем

diff --git a/kks/util/config.py b/kks/util/config.py
index 4c412e7..b3117c9 100644
--- a/kks/util/config.py
+++ b/kks/util/config.py
@@ -52,9 +52,9 @@ class Target:
         self.asm64bit = self.asm64bit if self.asm64bit is not None else default_target.asm64bit


-package_cfg = yaml.safe_load(resource_stream('kks', f'data/{target_file}'))
+package_cfg = None # yaml.safe_load(resource_stream('kks', f'data/{target_file}'))
 # package_default should have all fields set
-package_default = Target('default', package_cfg['default'])
+package_default = None # Target('default', package_cfg['default'])


 def check_version(cfg_file, cfg, is_global=False):
darkkeks:~/Documents/hse-homework/caos/sm06/4$ time kks top --help

real	0m0.517s
user	0m0.442s
sys	0m0.068s

Из-за чего это?

@darkkeks
Copy link
Owner

Из-за чего это?

pypa/setuptools#926

@vvd170501
Copy link
Collaborator Author

В init можно сделать импорт локальным.
Пока не совсем понятно, что можно сделать с kks/utils/config.py

@darkkeks
Copy link
Owner

Можно захардкодить файл строкой 🌚

@vvd170501
Copy link
Collaborator Author

Теперь шаблон конфига хранится в ~/.kks и (pkg_resources импортируется только по необходимости - когда нужно создать/обновить шаблон)

kks/cmd/init.py Outdated Show resolved Hide resolved
@vvd170501
Copy link
Collaborator Author

Насчёт импортов на уровне модуля возможно, стоит в будущем оптимизировать импорты requests и bs4 (в сумме занимают около 100мс, выполняются даже при запуске локальных команд (напр. `kks run`))

Тестировалось на vvd170501/kks@77f5201

$time kks run --help

import requests 0.051137447357177734
import requests 2.384185791015625e-07
import bs 0.05008339881896973
import h2t 0.002858400344848633
import requests 7.152557373046875e-07
import bs 2.384185791015625e-06
Usage: kks run [OPTIONS] [RUN_ARGS]...
...
real	0m0,192s
user	0m0,164s
sys	0m0,028s

@darkkeks
Copy link
Owner

darkkeks commented Jan 16, 2021

Если делать pip3 install . в директории с кодом, то не работает :(

Файла в site-packages действительно нету

Traceback (most recent call last):
  File "/usr/local/bin/kks", line 5, in <module>
    from kks.cli import cli
  File "/usr/local/lib/python3.9/site-packages/kks/cli.py", line 5, in <module>
    from kks.cmd.gen import gen
  File "/usr/local/lib/python3.9/site-packages/kks/cmd/gen.py", line 4, in <module>
    from kks.util.testing import TestSource, RunOptions
  File "/usr/local/lib/python3.9/site-packages/kks/util/testing.py", line 8, in <module>
    from kks.util.script import run_script, needs_compilation, compile_script
  File "/usr/local/lib/python3.9/site-packages/kks/util/script.py", line 6, in <module>
    from kks.binary import compile_cpp
  File "/usr/local/lib/python3.9/site-packages/kks/binary.py", line 8, in <module>
    from kks.util.config import find_target
  File "/usr/local/lib/python3.9/site-packages/kks/util/config.py", line 64, in <module>
    _copy_default(cfg_file)
  File "/usr/local/lib/python3.9/site-packages/kks/util/config.py", line 58, in _copy_default
    default_cfg = resource_stream('kks', f'data/{target_file}').read()
  File "/usr/local/lib/python3.9/site-packages/pkg_resources/__init__.py", line 1134, in resource_stream
    return get_provider(package_or_requirement).get_resource_stream(
  File "/usr/local/lib/python3.9/site-packages/pkg_resources/__init__.py", line 1606, in get_resource_stream
    return open(self._fn(self.module_path, resource_name), 'rb')
FileNotFoundError: [Errno 2] No such file or directory: '/usr/local/lib/python3.9/site-packages/kks/data/targets.yaml'

С --editable работает

@vvd170501
Copy link
Collaborator Author

vvd170501 commented Jan 16, 2021

Если делать pip3 install . в директории с кодом, то не работает :(

Странно, у меня нормально устанавливается и запускается (python3.8)
В virtualenv с python3.9 тоже всё работает.

Если сделать pip3 install --force-reinstall ., ошибка все равно остаётся?

@darkkeks
Copy link
Owner

Если сделать pip3 install --force-reinstall ., ошибка все равно остаётся?

Не помогло

Пробую на маке и на сервере с archlinux, и там и там python3.9, одинаково не работает

@vvd170501
Copy link
Collaborator Author

Исправил, проблема была в старом MANIFEST.in

@vvd170501 vvd170501 changed the title Поддержка различных вариантов сборки (beta) Поддержка различных вариантов сборки Jan 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants