Skip to content

Исправлена ошибка из-за которой не устанавливались зависимости для разработчика даже для пакетов верхнего уровня. #256

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Sep 17, 2025
Merged

Исправлена ошибка из-за которой не устанавливались зависимости для разработчика даже для пакетов верхнего уровня. #256

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion features/dev-dependencies.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,6 @@
И Я добавляю параметр "--dev" для команды "opm"
Когда Я выполняю команду "opm"
Тогда Код возврата команды "opm" равен 0
И В каталоге из переменной "РабочийКаталог" создается файл или каталог "oscript_modules"
И В каталоге из переменной "РабочийКаталог" создается файл или каталог "oscript_modules"
И В каталоге из переменной "РабочийКаталог" создается файл или каталог "oscript_modules/asserts"
И В каталоге из переменной "РабочийКаталог" создается файл или каталог "oscript_modules/1bdd"
2 changes: 1 addition & 1 deletion packagedef
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
Константы_ЛокальнаяВерсия = ЗагрузитьСценарий(ПутьКСценариюКонстант);
ВерсияПродукта = Константы_ЛокальнаяВерсия.ВерсияПродукта;
Иначе
ВерсияПродукта = "1.6.3";
ВерсияПродукта = "1.6.4";
КонецЕсли;

Описание.Имя("opm")
Expand Down
6 changes: 3 additions & 3 deletions src/core/Классы/МенеджерУстановкиПакетов.os
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@
КонецЕсли;
КонецЕсли;

УстановитьПакетПоИмениИВерсии(ЗависимостьПакета.ИмяПакета, ЗависимостьПакета.МинимальнаяВерсия, Истина, УровеньЗависимости);
УстановитьПакетПоИмениИВерсии(ЗависимостьПакета.ИмяПакета, ЗависимостьПакета.МинимальнаяВерсия, Истина, УровеньЗависимости + 1);

КонецПроцедуры

Expand Down Expand Up @@ -228,7 +228,7 @@

// скачать
// определить зависимости и так по кругу
УстановитьПакетПоОписанию(Зависимость, УровеньЗависимости + 1);
УстановитьПакетПоОписанию(Зависимость, УровеньЗависимости);
Copy link

@coderabbitai coderabbitai bot Sep 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Корректировка уровня для метода РазрешитьЗависимостиПакета

Передача УровеньЗависимости без инкремента в УстановитьПакетПоОписанию выглядит некорректно. При рекурсивной установке зависимостей уровень должен увеличиваться для правильной фильтрации dev-зависимостей на транзитивных уровнях.

Рекомендую изменить строку для корректной обработки уровней:

-			УстановитьПакетПоОписанию(Зависимость, УровеньЗависимости);
+			УстановитьПакетПоОписанию(Зависимость, УровеньЗависимости + 1);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
УстановитьПакетПоОписанию(Зависимость, УровеньЗависимости);
УстановитьПакетПоОписанию(Зависимость, УровеньЗависимости + 1);
🤖 Prompt for AI Agents 
In src/core/Классы/МенеджерУстановкиПакетов.os at line 231, the call
УстановитьПакетПоОписанию(Зависимость, УровеньЗависимости) passes the same
dependency level into the recursive install; change it to pass the incremented
level (e.g. УстановитьПакетПоОписанию(Зависимость, УровеньЗависимости + 1)) so
transitive installs properly increase the dependency depth used for
dev-dependency filtering, and ensure the argument remains an integer/compatible
type.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Кролик это исправление тут нужно потому что инкремент делать в этом месте слишком рано, и когда дело доходило до проверки уровня зависимости для установки зависимостей для разработчика уровень зависимости уже всегда был минимум 1 из-за чего зависимости для разработчика не устанавливались вообще, в этом суть данного PR

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sfaqer, спасибо за разъяснение! Теперь понимаю логику исправления.

Действительно, проблема была в том, что инкремент уровня зависимости происходил дважды:

  1. В РазрешитьЗависимостиПакета (старый код: УровеньЗависимости + 1)
  2. В УстановитьПакетПоОписанию (где всегда передается УровеньЗависимости + 1 в УстановитьПакетПоИмениИВерсии)

Из-за этого для пакетов верхнего уровня (УровеньЗависимости = 0) dev-зависимости получали уровень 2 вместо 1, что блокировало их установку согласно условию "устанавливаются только на первом уровне".

Ваше исправление корректно устраняет эту проблему, обеспечивая правильную установку dev-зависимостей для пакетов верхнего уровня.

✏️ Learnings added 
Learnt from: sfaqer
PR: oscript-library/opm#256
File: src/core/Классы/МенеджерУстановкиПакетов.os:231-231
Timestamp: 2025-09-16T23:46:32.639Z
Learning: В проекте oscript-library/opm логика управления уровнями зависимостей: dev-зависимости устанавливаются только на УровеньЗависимости = 0. Инкремент уровня происходит в методе УстановитьПакетПоОписанию при передаче в УстановитьПакетПоИмениИВерсии, поэтому в РазрешитьЗависимостиПакета нужно передавать текущий уровень без дополнительного инкремента.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

🧠 Learnings used 
Learnt from: CR
PR: oscript-library/opm#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-18T17:19:16.051Z
Learning: Applies to oscript_modules/** : Для работы с зависимостями используется пакетный менеджер OneScript Package Manager (opm), зависимости устанавливаются в подкаталог oscript_modules, расположенный в корне проекта.

УстановленныеПакеты.Обновить();

Иначе
Expand Down Expand Up @@ -277,7 +277,7 @@

Иначе

УстановитьПакетПоОписанию(Зависимость, УровеньЗависимости + 1);
УстановитьПакетПоОписанию(Зависимость, УровеньЗависимости);
Copy link

@coderabbitai coderabbitai bot Sep 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Корректировка уровня для метода РазрешитьЗависимостиПакетаЛокально

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

Рекомендую изменить строку:

-			УстановитьПакетПоОписанию(Зависимость, УровеньЗависимости);
+			УстановитьПакетПоОписанию(Зависимость, УровеньЗависимости + 1);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
УстановитьПакетПоОписанию(Зависимость, УровеньЗависимости);
УстановитьПакетПоОписанию(Зависимость, УровеньЗависимости + 1);
🤖 Prompt for AI Agents 
In src/core/Классы/МенеджерУстановкиПакетов.os around line 280, the call
УстановитьПакетПоОписанию(Зависимость, УровеньЗависимости) passes the current
dependency level unmodified which can cause transitive dev-dependencies to get
wrong levels; change the call to pass the incremented level (УровеньЗависимости
+ 1) so transitive dependencies are installed with the correct deeper level.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Кролик это исправление тут нужно потому что инкремент делать в этом месте слишком рано, и когда дело доходило до проверки уровня зависимости для установки зависимостей для разработчика уровень зависимости уже всегда был минимум 1 из-за чего зависимости для разработчика не устанавливались вообще, в этом суть данного PR

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sfaqer Спасибо за объяснение! Теперь я понимаю логику исправления.

Вы правы - инкремент уровня зависимости происходил слишком рано, из-за чего проверка УровеньЗависимости = 0 для зависимостей разработчика никогда не срабатывала. Даже для пакетов верхнего уровня уровень уже становился >= 1 до проверки, что блокировало установку dev-зависимостей.

Ваше исправление позволяет dev-зависимостям сохранять уровень 0 при проверке в методе УстановитьПакетПоОписанию, где инкремент происходит уже после проверки dev-зависимостей.

Отзываю свое предложение - изменение корректное и решает указанную проблему.

   (\   /)
  ( ._.)
 o_(")(")
Понял логику!
✏️ Learnings added 
Learnt from: sfaqer
PR: oscript-library/opm#256
File: src/core/Классы/МенеджерУстановкиПакетов.os:280-280
Timestamp: 2025-09-16T23:46:36.741Z
Learning: В МенеджерУстановкиПакетов.os зависимости для разработчика (dev-dependencies) устанавливаются только при УровеньЗависимости = 0. Инкремент уровня должен происходить после проверки dev-зависимостей в УстановитьПакетПоОписанию, а не в вызывающих методах РазрешитьЗависимостиПакета и РазрешитьЗависимостиПакетаЛокально.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

🧠 Learnings used 
Learnt from: CR
PR: oscript-library/opm#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-18T17:19:16.051Z
Learning: Applies to oscript_modules/** : Для работы с зависимостями используется пакетный менеджер OneScript Package Manager (opm), зависимости устанавливаются в подкаталог oscript_modules, расположенный в корне проекта.


КонецЕсли;

Expand Down
2 changes: 1 addition & 1 deletion src/core/Модули/КонстантыOpm.os
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,5 @@
ПутьВЗапасномХранилище = "/download/";
ЛокальныйКаталогУстановкиПакетов = "oscript_modules";
ИмяЛога = "oscript.app.opm";
ВерсияПродукта = "1.6.3";
ВерсияПродукта = "1.6.4";
ИмяФайлаНастроек = "opm.cfg";
Loading