Подпись для MacOS и немного рефакторинга #45
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
## Walkthrough
Внесены изменения в архитектуру определения окружения: глобальные переменные и прямые запросы к системной информации заменены на внедрение объекта "Окружение" (детектор окружения) во все ключевые классы и функции. Логика проверки платформы и архитектуры теперь централизована через этот объект, добавлены новые методы для работы с путями пользователя и локальными данными, а также реализована отдельная процедура подписи для macOS.
## Changes
| Файл(ы) | Краткое описание изменений |
|-------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------|
| src/core/Классы/ДетекторОкружения.os | Реорганизация: удалена функция ТипПлатформы, добавлены булевые методы для проверки платформы и архитектуры, новые методы путей. |
| src/core/Классы/ПараметрыOVM.os | Заменён глобальный объект СистемнаяИнформация на Окружение, удалена функция Это64БитнаяОперационнаяСистема, обновлён конструктор. |
| src/core/Классы/АктиваторOneScript.os | Внедрение Окружения вместо глобальных переменных, обновлены все проверки платформы и архитектуры, изменён конструктор. |
| src/core/Классы/ВерсииOneScript.os | Использование Окружения для проверки платформы, удалён глобальный ЭтоWindows, обновлён конструктор. |
| src/core/Классы/УстановщикOneScript.os | Внедрение Окружения, добавлена процедура ПодписатьOneScriptДляMacOS, обновлены проверки ОС, изменён конструктор. |
| src/core/Классы/ОпределительДистрибутива.os | Проверки платформы теперь через методы Окружения, логика разделена по платформам, сигнатуры не изменились. |
| src/cmd/Классы/КомандаInstall.os, src/cmd/Классы/КомандаRun.os | Внедрение Окружения, обновлены проверки платформы и архитектуры, изменены сигнатуры конструкторов. |
| tests/distrSelectorTests.os | Моки окружения теперь реализуют методы проверки платформы, архитектуры, удалены старые методы. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant Пользователь
participant Команда/Класс
participant ДетекторОкружения
Пользователь->>Команда/Класс: Создание объекта (Передаёт ДетекторОкружения)
Команда/Класс->>ДетекторОкружения: Вызов метода (например, ЭтоWindows(), ЭтоX64())
ДетекторОкружения-->>Команда/Класс: Возвращает результат проверки платформы/архитектуры
Команда/Класс-->>Пользователь: Выполняет логику с учётом окруженияsequenceDiagram
participant УстановщикOneScript
participant ДетекторОкружения
participant ОС
УстановщикOneScript->>ДетекторОкружения: ЭтоMacOS()
alt Если macOS
УстановщикOneScript->>ОС: Выполнить codesign ./oscript
ОС-->>УстановщикOneScript: Лог выполнения подписи
end
Possibly related PRs
Suggested reviewers
Poem
Learnt from: sfaqer |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/core/Классы/ДетекторОкружения.os (1)
37-50: Оптимизировать проверку архитектуры MacOS ARM.Текущая реализация неэффективна: для платформ, отличных от macOS, происходит лишний вызов
ЭтоMacOS(), а для macOS x86/x86_64 выполняется ненужная проверка архитектуры процессора.Рассмотрите более эффективную реализацию:
Функция ЭтоMacOSARM() Экспорт - Если Не ЭтоMacOS() Тогда + Если ТекущаяПлатформа <> ТипПлатформы.MacOS_x86_64 И ТекущаяПлатформа <> ТипПлатформы.MacOS_x86 Тогда Возврат Ложь; КонецЕсли; АрхитектураПроцессора = ПровайдерИнформацииОПроцессоре .Достать() .Архитектура; Возврат АрхитектураПроцессора = АрхитектурыПроцессоров.ARM Или АрхитектураПроцессора = АрхитектурыПроцессоров.ARM64; КонецФункции
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/cmd/Классы/КомандаInstall.os(3 hunks)src/cmd/Классы/КомандаRun.os(2 hunks)src/core/Классы/АктиваторOneScript.os(10 hunks)src/core/Классы/ВерсииOneScript.os(4 hunks)src/core/Классы/ДетекторОкружения.os(2 hunks)src/core/Классы/ОпределительДистрибутива.os(1 hunks)src/core/Классы/ПараметрыOVM.os(2 hunks)src/core/Классы/УстановщикOneScript.os(5 hunks)tests/distrSelectorTests.os(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (18)
- GitHub Check: test (stable, 1testrunner) / build (windows-latest, stable, 1testrunner)
- GitHub Check: test (stable, 1testrunner) / build (ubuntu-latest, stable, 1testrunner)
- GitHub Check: test (default, 1testrunner) / build (ubuntu-latest, default, 1testrunner)
- GitHub Check: test (default, 1testrunner) / build (windows-latest, default, 1testrunner)
- GitHub Check: test (dev, oneunit) / build (windows-latest, dev, oneunit)
- GitHub Check: test (dev, 1testrunner) / build (windows-latest, dev, 1testrunner)
- GitHub Check: test (dev, oneunit) / build (ubuntu-latest, dev, oneunit)
- GitHub Check: test (dev, 1testrunner) / build (ubuntu-latest, dev, 1testrunner)
- GitHub Check: test (dev, oneunit) / build (macos-latest, dev, oneunit)
- GitHub Check: test (dev, 1testrunner) / build (macos-latest, dev, 1testrunner)
- GitHub Check: test (dev, 1testrunner) / build (ubuntu-latest, dev, 1testrunner)
- GitHub Check: test (dev, 1testrunner) / build (windows-latest, dev, 1testrunner)
- GitHub Check: test (stable, 1testrunner) / build (macos-latest, stable, 1testrunner)
- GitHub Check: test (stable, 1testrunner) / build (ubuntu-latest, stable, 1testrunner)
- GitHub Check: test (dev, oneunit) / build (macos-latest, dev, oneunit)
- GitHub Check: test (default, 1testrunner) / build (macos-latest, default, 1testrunner)
- GitHub Check: test (default, 1testrunner) / build (ubuntu-latest, default, 1testrunner)
- GitHub Check: test (dev, oneunit) / build (ubuntu-latest, dev, oneunit)
🔇 Additional comments (22)
src/cmd/Классы/КомандаInstall.os (2)
9-9: Правильное внедрение зависимости для детектора окруженияОтличное решение по замене глобальных переменных на внедрение зависимости через конструктор. Это улучшает модульность и тестируемость кода.
Also applies to: 12-13
30-30: Корректная замена метода проверки архитектурыЗамена
ПараметрыOVM.Это64БитнаяОперационнаяСистема()наОкружение.ЭтоX64()логична и консистентна с новой архитектурой определения окружения.Also applies to: 68-68
tests/distrSelectorTests.os (1)
276-284: Правильное обновление мока под новую архитектуруОбновление функции
МокОкружениякорректно отражает переход от прямого возвращения типа платформы к boolean методам. Логика определения платформ черезСтрНачинаетсяСи дополнительная проверка архитектуры дляЭтоMacOSARM()реализованы правильно.src/core/Классы/ВерсииOneScript.os (2)
16-16: Консистентное внедрение детектора окруженияПравильное добавление переменной
Окружениеи обновление конструктора для внедрения зависимости. Паттерн соответствует изменениям в других классах.Also applies to: 19-21
121-121: Корректная замена глобальной переменной на метод объектаВсе места использования глобальной переменной
ЭтоWindowsправильно заменены на вызов методаОкружение.ЭтоWindows(). Это улучшает инкапсуляцию и тестируемость кода.Also applies to: 123-123, 178-178, 190-190
src/cmd/Классы/КомандаRun.os (2)
26-26: Успешное применение паттерна внедрения зависимостиДобавление переменной
Окружениеи обновление конструктора полностью соответствует паттерну, применяемому в других классах проекта.Also applies to: 29-31
41-41: Правильная замена проверки платформыЗамена прямого обращения к глобальной переменной на вызов метода объекта окружения улучшает модульность кода и соответствует новой архитектуре.
src/core/Классы/АктиваторOneScript.os (3)
19-21: Отличная реализация внедрения зависимости.Конструктор корректно принимает детектор окружения и сохраняет его для использования в классе. Это хорошая архитектурная практика.
66-66: LGTM: Корректная замена прямых вызовов системной информации.Все обращения к определению платформы правильно заменены на использование внедренного объекта окружения.
Also applies to: 81-81, 110-110, 122-122, 131-131, 146-146, 159-159, 221-221
223-227: Хорошее использование новых методов окружения.Использование
ПутьКПрофилюПользователя()иЭтоX64()из объекта окружения улучшает инкапсуляцию и тестируемость кода.Also applies to: 246-246, 264-264
src/core/Классы/ПараметрыOVM.os (2)
9-11: Корректная реализация паттерна внедрения зависимости.Конструктор правильно принимает и сохраняет детектор окружения для использования в классе.
24-24: Правильное использование метода окружения для получения пути.Замена прямого обращения к системной информации на использование
ПутьКЛокальнымДаннымПриложений()улучшает архитектуру кода.src/core/Классы/УстановщикOneScript.os (3)
25-27: Корректная реализация внедрения зависимости.Конструктор правильно принимает детектор окружения согласно общему паттерну рефакторинга.
159-159: LGTM: Правильная замена проверок платформы.Использование
Окружение.ЭтоWindows()вместо прямых обращений к глобальным переменным улучшает архитектуру.Also applies to: 221-221
101-101: Хорошее добавление подписи для macOS.Вызов процедуры подписи в правильном месте процесса установки обеспечивает корректную работу OneScript на macOS.
src/core/Классы/ДетекторОкружения.os (7)
4-4: Переменная ТекущаяПлатформа правильно инициализирована.Добавление кэширования типа платформы в переменную экземпляра - хорошее решение для производительности.
Also applies to: 13-13
16-21: Функция ЭтоWindows() корректно реализована.Функция правильно проверяет обе архитектуры Windows (x86 и x86_64).
23-28: Функция ЭтоLinux() корректно реализована.Функция правильно проверяет обе архитектуры Linux (x86 и x86_64).
30-35: Функция ЭтоMacOS() корректно реализована.Функция правильно проверяет обе архитектуры macOS (x86 и x86_64).
52-54: Функция ПутьКПрофилюПользователя() корректно реализована.Простая и понятная реализация получения пути к профилю пользователя.
56-58: Функция ПутьКЛокальнымДаннымПриложений() корректно реализована.Простая и понятная реализация получения пути к локальным данным приложений.
60-62: Функция ЭтоX64() корректно реализована.Использование системного свойства
Это64БитнаяОперационнаяСистемаявляется правильным подходом для определения архитектуры ОС.
sfaqer
force-pushed
the
bugfix/MacOSCodeSign
branch
from
15ca1dd to
f305711
Compare
2. Работа с окружением инкапсулирована внутри ДетекторОкружения
sfaqer
force-pushed
the
bugfix/MacOSCodeSign
branch
from
f305711 to
7ece325
Compare
There was a problem hiding this comment.
Pull Request Overview
This pull request implements environment encapsulation by introducing a dedicated ДетекторОкружения and updating various modules (installation, activation, tests, command-line interfaces) to rely on its methods instead of global variables. Key changes include:
- Replacing global platform and architecture checks with calls to methods like ЭтоWindows(), ЭтоLinux(), ЭтоMacOS(), and ЭтоMacOSARM().
- Updating tests to mock the new methods and adjusting command and installation procedures accordingly.
- Adding a new procedure for signing OneScript on macOS after installation.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/distrSelectorTests.os | Updated mocks to use the new environment detection methods |
| src/core/Классы/УстановщикOneScript.os | Integrated environment object and added signing for macOS |
| src/core/Классы/ПараметрыOVM.os | Replaced SистемнаяИнформация with the encapsulated Окружение |
| src/core/Классы/ОпределительДистрибутива.os | Updated distro type determination based on environment |
| src/core/Классы/ДетекторОкружения.os | Introduced new functions to detect platform types and paths |
| src/core/Классы/ВерсииOneScript.os | Updated executable path resolution using Окружение |
| src/core/Классы/АктиваторOneScript.os | Replaced global OS checks with Окружение calls |
| src/cmd/Классы/КомандаRun.os | Adjusted PATH handling to use the Окружение methods |
| src/cmd/Классы/КомандаInstall.os | Replaced bit-checks and OS checks with Окружение usage |
Comments suppressed due to low confidence (1)
tests/distrSelectorTests.os:281
- Consider adding explicit parentheses in the combined logical expression for clarity, for example: СтрНачинаетсяС(ВидПлатформы, "MacOS") И (Архитектура = АрхитектурыПроцессоров.ARM64 Или Архитектура = АрхитектурыПроцессоров.ARM).
МокСИ.Когда().ЭтоMacOSARM().ТогдаВозвращает(СтрНачинаетсяС(ВидПлатформы, "MacOS") И Архитектура = АрхитектурыПроцессоров.ARM64 Или Архитектура = АрхитектурыПроцессоров.ARM);
|
|
||
| Команда = Новый Команда; | ||
|
|
||
| Команда.УстановитьКоманду("codesign"); |
There was a problem hiding this comment.
It may be beneficial to add explicit error handling for the codesign process so that failures provide clear diagnostic information, even though a proper return code is already being set.
Copilot uses AI. Check for mistakes.
nixel2007
reviewed
Jun 8, 2025
|
|
||
| Команда = Новый Команда; | ||
|
|
||
| Команда.УстановитьКоманду("codesign"); |
There was a problem hiding this comment.
Я не знаю, у меняж мака нет, но в GA работает, у пользователей тоже.
Предположу что да.
nixel2007
reviewed
Jun 8, 2025
|
|
||
| Команда.УстановитьПравильныйКодВозврата(0); | ||
|
|
||
| Команда.Исполнить(); |
There was a problem hiding this comment.
Может обработку ошибок добавить с внятным текстом исключения? Чтобы было понятно, куда бежать
There was a problem hiding this comment.
А чем тебе не внятная ошибка?
There was a problem hiding this comment.
Ты в контексте. Я смотрю с точки зрения пользователя. Вот видит он такое или что подпись не удалась. Что? Какая подпись? Куда бежать и что делать? Оно будет работать? Или надо руками что-то подшаманить?
There was a problem hiding this comment.
Ну, во всех случаях просто несёшь это в ишью\чат оскрипта, не? Просто потому что если ты не знаешь чё такое codesign то и моё сообщение "Codesign не смог подписать" тебе тоже ничем не поможет
There was a problem hiding this comment.
Ну т.е ты какой текст исключения считаешь "внятным" ?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Новые функции
Исправления
Тесты