Skip to content

Conversation

@yungwine
Copy link
Collaborator

@yungwine yungwine commented Sep 8, 2025

No description provided.

@yungwine yungwine requested a review from Copilot September 8, 2025 11:24
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for specifying a custom TON node git repository URL during installation and upgrades, replacing the hardcoded GitHub repository with a configurable option.

  • Added -g parameter to installer and upgrade scripts to accept custom git URLs
  • Modified configuration handling to support URL-based installation alongside existing author/repo pattern
  • Updated help text and parameter processing across multiple scripts

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
scripts/ton_installer.sh Added -g flag support and variable for custom git URL
scripts/install.sh Added git URL parameter handling and config override logic
mytonctrl/scripts/upgrade.sh Added git URL support for upgrade operations
mytonctrl/mytonctrl.py Enhanced check_git function to handle URL-based upgrades with fragment parsing

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines 113 to +114
rm -rf $SOURCES_DIR/ton
git clone --recursive https://github.com/ton-blockchain/ton.git
git clone --recursive $repo_git_url $SOURCES_DIR/ton
Copy link

Copilot AI Sep 8, 2025

Choose a reason for hiding this comment

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

The git clone command should specify the target directory as the last argument after the URL. Currently it's cloning to $SOURCES_DIR/ton which creates a nested directory structure. It should be git clone --recursive $repo_git_url ton and then cd ton.

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Choose a reason for hiding this comment

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

We force clone into $SOURCES_DIR/ton to keep a deterministic path since repo names may vary.

Comment on lines +341 to 344
author, repo, branch, _ = check_git(args, repo, "update") # todo: implement --url for update
# Run script
update_script_path = pkg_resources.resource_filename('mytonctrl', 'scripts/update.sh')
runArgs = ["bash", update_script_path, "-a", author, "-r", repo, "-b", branch]
Copy link

Copilot AI Sep 8, 2025

Choose a reason for hiding this comment

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

The TODO comment indicates incomplete implementation. The Update function doesn't support the --url parameter that was added to check_git, creating inconsistent functionality between update and upgrade operations.

Suggested change
author, repo, branch, _ = check_git(args, repo, "update") # todo: implement --url for update
# Run script
update_script_path = pkg_resources.resource_filename('mytonctrl', 'scripts/update.sh')
runArgs = ["bash", update_script_path, "-a", author, "-r", repo, "-b", branch]
author, repo, branch, git_url = check_git(args, repo, "update") # implemented --url for update
# Run script
update_script_path = pkg_resources.resource_filename('mytonctrl', 'scripts/update.sh')
if git_url is not None:
runArgs = ["bash", update_script_path, "-u", git_url, "-b", branch]
else:
runArgs = ["bash", update_script_path, "-a", author, "-r", repo, "-b", branch]

Copilot uses AI. Check for mistakes.
Comment on lines +288 to +292
if '#' in git_url:
ref_fragment = git_url.rsplit('#', 1)[1]
if not ref_fragment:
raise Exception("--url fragment after # is empty")
branch = ref_fragment
Copy link

Copilot AI Sep 8, 2025

Choose a reason for hiding this comment

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

The fragment parsing doesn't validate the extracted branch name. This could allow injection of arbitrary git references that might not be safe branch/tag names.

Copilot uses AI. Check for mistakes.
repo = "mytonctrl"
author, repo, branch = check_git(args, repo, "update")

author, repo, branch, _ = check_git(args, repo, "update") # todo: implement --url for update
Copy link
Contributor

Choose a reason for hiding this comment

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

@yungwine I propose to remove # todo: implement --url for update as whole idea of that PR to keep upstream MTC for custom TON nodes.

@yungwine yungwine merged commit ac7fa24 into ton-blockchain:dev Sep 9, 2025
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