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

Mercurial command server #459

Open
wants to merge 28 commits into
base: master
Choose a base branch
from

Conversation

f-fr
Copy link
Contributor

@f-fr f-fr commented Jan 3, 2021

This is a modification of the HgResolver from some other pull request to make use of the hg command server to speed up hg commands.

The mercurial command server is feature of hg to overcome the (relatively long) startup time of the Python interpreter. Instead of starting a new interpreter for each single hg command only a single process is started. All subsequent hg commands are then passed to this running process.

The command server is started by HgResolver the first time a command is run. The same command server is also used for all test cases.

This overall reduces the running time of all hg tests to a few seconds.

Copy link
Contributor

@Sija Sija left a comment

Choose a reason for hiding this comment

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

Couple of idiomatic suggestions.


protected def versions_from_tags
capture_hg("tags", "--template", "{tag}\\n")
.split('\n')
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.split('\n')
.lines

return HgTagRef.new value
when "commit"
return HgCommitRef.new value
else
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
else

source = hg_url
# Remove a "file://" from the beginning, otherwise the path might be invalid
# on Windows.
source = source[7..] if source.starts_with?("file://")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
source = source[7..] if source.starts_with?("file://")
source = source.lchop("file://")

Comment on lines 353 to 354
yield
break
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
yield
break
return yield


private def valid_repository?
File.each_line(File.join(local_path, ".hg", "dirstate")) do |line|
return true if line =~ /mirror\s*=\s*true/
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return true if line =~ /mirror\s*=\s*true/
return true if line.matches?(/mirror\s*=\s*true/)

@straight-shoota
Copy link
Member

Please let's focus on #458 first and revisit this once that is merged.

@straight-shoota
Copy link
Member

straight-shoota commented Sep 20, 2021

#458 has been merged, so we can move forward with this.

Sorry for the slow review process 🙇‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants