Avoid modifying the Mercurial repository config file to add config options

Modifying the config file is complicated and error prone, but there hasn't been
any other option aside from setting `HGRCPATH` to point to a standalone file.
Starting with Mercurial 7.0 (scheduled for March 2025), there's now a global
option to specify one or more additional config files on the command line,
without disabling the normal system and user level config file processing.[1]
Since I'm not sure what the minimum supported Mercurial is for this project,
this includes an extension that backports the same option if it is not present
in the version of Mercurial that is used, and does nothing if Mercurial natively
supports it.  I tested back to Mercurial 6.0, which should be more than
sufficient- 6.1.4 (June 2022) was the last release to support Python 2 (which
has been EOL since Jan 2020), and the Python 3 support before that release was
considered experimental.  It likely works in earlier versions, but there's a
definite minimum of 4.9 (Feb 2019), due to the `exthelper` module import.

Without the need to modify a possibly existing file and then restore it when
done, a bunch of code falls away, and the tests that supported it.


[1] https://repo.mercurial-scm.org/hg/rev/25b344f2aeef
This commit is contained in:
Matt Harbison
2025-02-07 16:16:24 -05:00
parent 0fa7ddfb9d
commit c30a06d26a
8 changed files with 233 additions and 160 deletions

View File

@@ -28,7 +28,8 @@ public enum HgExtensions {
HOOK("scmhooks.py"),
CGISERVE("cgiserve.py"),
VERSION("scmversion.py"),
FILEVIEW("fileview.py");
FILEVIEW("fileview.py"),
CONFIGFILE("configfile.py");
private static final String BASE_DIRECTORY = "lib".concat(File.separator).concat("python");
private static final String BASE_RESOURCE = "/sonia/scm/python/";

View File

@@ -23,6 +23,7 @@ import org.javahg.ext.purge.PurgeExtension;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import sonia.scm.repository.hooks.HookEnvironment;
import sonia.scm.repository.spi.javahg.HgConfigFileExtension;
import sonia.scm.repository.spi.javahg.HgFileviewExtension;
import java.io.File;
@@ -64,6 +65,7 @@ public class HgRepositoryFactory {
repoConfiguration.getEnvironment().putAll(environment);
repoConfiguration.addExtension(HgFileviewExtension.class);
repoConfiguration.addExtension(PurgeExtension.class);
repoConfiguration.addExtension(HgConfigFileExtension.class);
boolean pending = hookEnvironment.isPending();
repoConfiguration.setEnablePendingChangesets(pending);

View File

@@ -29,6 +29,7 @@ import sonia.scm.repository.api.ImportFailedException;
import sonia.scm.repository.api.PullResponse;
import java.io.IOException;
import java.nio.file.Path;
import java.util.List;
import static sonia.scm.ContextEntry.ContextBuilder.entity;
@@ -71,7 +72,15 @@ public class HgPullCommand extends AbstractHgPushOrPullCommand implements PullCo
List<Changeset> result;
try {
result = builder.call(() -> org.javahg.commands.PullCommand.on(open()).execute(url));
result = builder.call((Path configFile) -> {
org.javahg.commands.PullCommand cmd = org.javahg.commands.PullCommand.on(open());
if(configFile != null) {
cmd.cmdAppend("--config-file", configFile.toFile().getAbsolutePath());
}
return cmd.execute(url);
});
} catch (ExecutionException ex) {
throw new ImportFailedException(entity(getRepository()).build(), "could not execute pull command", ex);
}

View File

@@ -29,6 +29,7 @@ import sonia.scm.repository.api.PushFailedException;
import sonia.scm.repository.api.PushResponse;
import java.io.IOException;
import java.nio.file.Path;
import java.util.List;
@@ -59,13 +60,17 @@ public class HgPushCommand extends AbstractHgPushOrPullCommand implements PushCo
List<Changeset> result;
try {
result = builder.call(() -> {
result = builder.call((Path configFile) -> {
org.javahg.commands.PushCommand hgPush = org.javahg.commands.PushCommand.on(open());
if (request.isForce()) {
hgPush.force();
}
if(configFile != null) {
hgPush.cmdAppend("--config-file", configFile.toFile().getAbsolutePath());
}
return hgPush.execute(url);
});
} catch (ExecutionException ex) {

View File

@@ -22,7 +22,6 @@ import jakarta.inject.Inject;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import sonia.scm.io.INIConfiguration;
import sonia.scm.io.INIConfigurationReader;
import sonia.scm.io.INIConfigurationWriter;
import sonia.scm.io.INISection;
import sonia.scm.net.GlobalProxyConfiguration;
@@ -32,6 +31,9 @@ import sonia.scm.util.Util;
import java.io.File;
import java.io.IOException;
import java.net.URI;
import java.nio.file.Files;
import java.nio.file.Path;
public class TemporaryConfigFactory {
@@ -59,9 +61,6 @@ public class TemporaryConfigFactory {
private String username;
private String password;
private INIConfiguration hgrc;
private INISection previousProxyConfiguration;
private Builder(HgCommandContext context) {
this.context = context;
}
@@ -75,40 +74,29 @@ public class TemporaryConfigFactory {
@SuppressWarnings("java:S4042") // we know that we delete a file
public <T> T call(HgCallable<T> callable) throws IOException {
File file = new File(context.getDirectory(), HgRepositoryHandler.PATH_HGRC);
boolean exists = file.exists();
Path hgrc = null;
if (isModificationRequired()) {
setupHgrc(file);
hgrc = Files.createTempFile(null, ".rc");
}
try {
return callable.call();
if (hgrc != null) {
setupHgrc(hgrc.toFile());
}
return callable.call(hgrc);
} finally {
if (!exists && file.exists() && !file.delete()) {
LOG.error("failed to delete temporary hgrc {}", file);
} else if (exists && file.exists()) {
try {
if (hgrc != null) {
cleanUpHgrc(file);
}
} catch (Exception e) {
LOG.warn("error cleaning up hgrc", e);
if (hgrc != null) {
if(!hgrc.toFile().delete()) {
LOG.warn("error cleaning up hgrc {}", hgrc.toFile().getAbsoluteFile());
}
}
}
}
private void write(File file) throws IOException {
INIConfigurationWriter writer = new INIConfigurationWriter();
writer.write(hgrc, file);
}
private void setupHgrc(File file) throws IOException {
if (file.exists()) {
INIConfigurationReader reader = new INIConfigurationReader();
hgrc = reader.read(file);
} else {
hgrc = new INIConfiguration();
}
INIConfiguration hgrc = new INIConfiguration();
if (isAuthenticationEnabled()) {
applyAuthentication(hgrc);
@@ -118,12 +106,11 @@ public class TemporaryConfigFactory {
applyProxyConfiguration(hgrc);
}
write(file);
INIConfigurationWriter writer = new INIConfigurationWriter();
writer.write(hgrc, file);
}
private void applyProxyConfiguration(INIConfiguration hgrc) {
previousProxyConfiguration = hgrc.getSection(SECTION_PROXY);
hgrc.removeSection(SECTION_PROXY);
INISection proxy = new INISection(SECTION_PROXY);
proxy.setParameter("host", globalProxyConfiguration.getHost() + ":" + globalProxyConfiguration.getPort());
@@ -165,34 +152,11 @@ public class TemporaryConfigFactory {
&& !Strings.isNullOrEmpty(username)
&& !Strings.isNullOrEmpty(password);
}
private void cleanUpHgrc(File file) throws IOException {
INISection auth = hgrc.getSection(SECTION_AUTH);
if (isAuthenticationEnabled() && auth != null) {
for (String key : auth.getParameterKeys()) {
if (key.startsWith(AUTH_PREFIX)) {
auth.removeParameter(key);
}
}
}
if (globalProxyConfiguration.isEnabled()) {
hgrc.removeSection(SECTION_PROXY);
if (previousProxyConfiguration != null) {
hgrc.addSection(previousProxyConfiguration);
}
}
if (isModificationRequired()) {
write(file);
}
}
}
@FunctionalInterface
public interface HgCallable<T> {
T call() throws IOException;
T call(Path configFile) throws IOException;
}
}

View File

@@ -0,0 +1,35 @@
/*
* Copyright (c) 2025 - present Cloudogu GmbH
*
* This program is free software: you can redistribute it and/or modify it under
* the terms of the GNU Affero General Public License as published by the Free
* Software Foundation, version 3.
*
* This program is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
* FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
* details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see https://www.gnu.org/licenses/.
*/
package sonia.scm.repository.spi.javahg;
import org.javahg.MercurialExtension;
import sonia.scm.repository.HgExtensions;
public class HgConfigFileExtension extends MercurialExtension {
static final String NAME = "configfile";
@Override
public String getName() {
return NAME;
}
@Override
public String getPath() {
return HgExtensions.CONFIGFILE.getFile().getAbsolutePath();
}
}

View File

@@ -0,0 +1,144 @@
#
# Copyright (c) 2025 - present Cloudogu GmbH
#
# This program is free software: you can redistribute it and/or modify it under
# the terms of the GNU Affero General Public License as published by the Free
# Software Foundation, version 3.
#
# This program is distributed in the hope that it will be useful, but WITHOUT
# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
# FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
# details.
#
# You should have received a copy of the GNU Affero General Public License
# along with this program. If not, see https://www.gnu.org/licenses/.
#
"""Add a ``--config-file`` option, prior to support for it in Mercurial 7.0
This extension wraps every command with processing for one or more optional
``--config-file HGRC`` arguments. Like the core functionality introduced in hg
7.0, the configuration stored in these files overwrites any of the same config
options that were stored in system, user, or repo level config files. Only
``--config`` options will take priority over the config options stored in these
files. If multiple files are specified, the config in the last file overrides
the config in any previous file.
The limitation with this extension is that it can't introduce a truly global
option like hg 7.0 has. Therefore, the option must be given after the command
verb. Mercurial 7.0 can process this option before or after the verb. This
extension detects if Mercurial already supports the option, and does nothing in
that case.
"""
from mercurial import (
commands,
config as configmod,
error,
exthelper,
extensions,
pycompat,
)
from mercurial.i18n import _
# 6.0 was released on Nov 2021; 6.1.4 is the last Python 2 release.
testedwith = b'6.0 6.0.3 6.1.4 6.2.3 6.3.3 6.4.5 6.5.3 6.6.3 6.7.4 6.8.2 6.9.1'
eh = exthelper.exthelper()
extsetup = eh.finalextsetup
@eh.extsetup
def _extsetup(ui):
# Mercurial started providing a global --config-file option starting with
# 7.0. The command line API is guaranteed to be stable over time, but the
# internal APIs are not. Therefore, look for the global option before doing
# anything, and bail out of the setup if it is present. If something goes
# wrong with the search, assume that something was refactored in a post-7.0
# world, and also skip the setup since it will be supported natively.
try:
for opt in commands.globalopts:
if opt[1] == b'config-file':
return
except (AttributeError, IndexError):
return
# There is no global option, so patch all commands to add --config-file.
# Unlike a global option (which can appear anywhere), this must appear
# after the command verb.
for cmd, impl in commands.table.items():
# Some commands have aliases appended with a '|', and can't be wrapped
# with that name.
if b'|' in cmd:
cmd = cmd.split(b'|')[0]
addopt = [
(
b'',
b'config-file',
[],
b'load config file to set/override config options',
b'HGRC',
)
]
# Wrap the command function to handle the option
entry = extensions.wrapcommand(commands.table, cmd, _wrapper)
entry[1].extend(addopt)
def _wrapper(orig, ui, *args, **kwargs):
configs = _parse_config_files(pycompat.sysargv, kwargs.pop('config_file'))
overrides = {}
# The --config option overrides --config-file contents in the global parser.
# But since that's all been populated in the ui object at this point, take
# care to not replace those items.
for section, name, value, src in configs:
if ui.configsource(section, name) != b'--config':
overrides[(section, name)] = value
with ui.configoverride(overrides, b"--config-file"):
return orig(ui, *args, **kwargs)
def _parse_config_files(cmdargs, config_files):
"""parse the --config-file options from the command line
A list of tuples containing (section, name, value, source) is returned,
in the order they were read.
"""
configs = []
cfg = configmod.config()
for file in config_files:
try:
cfg.read(file)
except error.ConfigError as e:
raise error.InputError(
_(b'invalid --config-file content at %s') % e.location,
hint=e.message,
)
except FileNotFoundError:
hint = None
if b'--cwd' in cmdargs:
hint = _(b"this file is resolved before --cwd is processed")
raise error.InputError(
_(b'missing file "%s" for --config-file') % file, hint=hint
)
for section in cfg.sections():
for item in cfg.items(section):
name = item[0]
value = item[1]
src = cfg.source(section, name)
configs.append((section, name, value, src))
return configs

View File

@@ -26,16 +26,13 @@ import org.mockito.junit.jupiter.MockitoExtension;
import sonia.scm.config.ScmConfiguration;
import sonia.scm.io.INIConfiguration;
import sonia.scm.io.INIConfigurationReader;
import sonia.scm.io.INIConfigurationWriter;
import sonia.scm.io.INISection;
import sonia.scm.net.GlobalProxyConfiguration;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.lenient;
@ExtendWith(MockitoExtension.class)
class TemporaryConfigFactoryTest {
@@ -45,40 +42,28 @@ class TemporaryConfigFactoryTest {
private TemporaryConfigFactory configFactory;
private ScmConfiguration configuration;
private Path hgrc;
@BeforeEach
void setUp(@TempDir Path directory) throws IOException {
Path hg = Files.createDirectories(directory.resolve(".hg"));
hgrc = hg.resolve("hgrc");
lenient().when(commandContext.getDirectory()).thenReturn(directory.toFile());
configuration = new ScmConfiguration();
configFactory = new TemporaryConfigFactory(new GlobalProxyConfiguration(configuration));
}
@Test
void shouldNotCreateHgrc() throws IOException {
configFactory.withContext(commandContext).call(() -> {
assertThat(hgrc).doesNotExist();
configFactory.withContext(commandContext).call((Path configFile) -> {
assertThat(configFile).doesNotExist();
return null;
});
}
@Test
@SuppressWarnings("java:S2699") // test should just ensure that no exception is thrown
void shouldNotFailIfFileExistsButWithoutProxyOrAuthentication() throws IOException {
Files.createFile(hgrc);
configFactory.withContext(commandContext).call(() -> null);
}
@Test
void shouldCreateHgrcWithAuthentication() throws IOException {
configFactory
.withContext(commandContext)
.withCredentials("https://hg.hitchhiker.org/repo", "trillian", "secret")
.call(() -> {
INIConfiguration ini = assertHgrc();
.call((Path configFile) -> {
INIConfiguration ini = assertHgrc(configFile);
INISection auth = ini.getSection("auth");
assertThat(auth).isNotNull();
@@ -91,7 +76,7 @@ class TemporaryConfigFactoryTest {
});
}
private INIConfiguration assertHgrc() throws IOException {
private INIConfiguration assertHgrc(Path hgrc) throws IOException {
assertThat(hgrc).exists();
INIConfigurationReader reader = new INIConfigurationReader();
@@ -103,8 +88,8 @@ class TemporaryConfigFactoryTest {
configuration.setEnableProxy(true);
configuration.setProxyServer("proxy.hitchhiker.org");
configuration.setProxyPort(3128);
configFactory.withContext(commandContext).call(() -> {
INIConfiguration ini = assertHgrc();
configFactory.withContext(commandContext).call((Path configFile) -> {
INIConfiguration ini = assertHgrc(configFile);
INISection proxy = ini.getSection("http_proxy");
assertThat(proxy).isNotNull();
@@ -121,8 +106,8 @@ class TemporaryConfigFactoryTest {
configuration.setProxyUser("trillian");
configuration.setProxyPassword("secret");
configFactory.withContext(commandContext).call(() -> {
INIConfiguration ini = assertHgrc();
configFactory.withContext(commandContext).call((Path configFile) -> {
INIConfiguration ini = assertHgrc(configFile);
INISection proxy = ini.getSection("http_proxy");
assertThat(proxy).isNotNull();
@@ -140,8 +125,8 @@ class TemporaryConfigFactoryTest {
configuration.setProxyPort(3128);
configuration.setProxyExcludes(ImmutableSet.of("hg.hitchhiker.org", "localhost", "127.0.0.1"));
configFactory.withContext(commandContext).call(() -> {
INIConfiguration ini = assertHgrc();
configFactory.withContext(commandContext).call((Path configFile) -> {
INIConfiguration ini = assertHgrc(configFile);
INISection proxy = ini.getSection("http_proxy");
assertThat(proxy).isNotNull();
@@ -152,85 +137,13 @@ class TemporaryConfigFactoryTest {
@Test
void shouldRemoveCreatedHgrc() throws IOException {
configFactory
Path hgrc = configFactory
.withContext(commandContext)
.withCredentials("https://hg.hitchhiker.com", "marvin", "brainLikeAPlanet")
.call(() -> {
assertThat(hgrc).exists();
return null;
.call((Path configFile) -> {
assertThat(configFile).exists();
return configFile;
});
assertThat(hgrc).doesNotExist();
}
@Test
void shouldKeepAuthenticationInformation() throws IOException {
writeAuthentication();
configFactory
.withContext(commandContext)
.withCredentials("https://hg.hitchhiker.com", "marvin", "brainLikeAPlanet")
.call(() -> {
INIConfiguration configuration = assertHgrc();
INISection auth = configuration.getSection("auth");
assertThat(auth).isNotNull();
assertThat(auth.getParameter("a.username")).isEqualTo("dent");
assertThat(auth.getParameter("temporary.username")).isEqualTo("marvin");
return null;
});
INIConfiguration configuration = assertHgrc();
INISection auth = configuration.getSection("auth");
assertThat(auth).isNotNull();
assertThat(auth.getParameter("a.username")).isEqualTo("dent");
assertThat(auth.getParameter("temporary.username")).isNull();
}
@Test
void shouldRestoreProxyConfiguration() throws IOException {
writeProxyConfiguration();
configuration.setEnableProxy(true);
configuration.setProxyServer("proxy.hitchhiker.org");
configuration.setProxyPort(3128);
configFactory.withContext(commandContext).call(() -> {
INIConfiguration ini = assertHgrc();
INISection proxy = ini.getSection("http_proxy");
assertThat(proxy).isNotNull();
assertThat(proxy.getParameter("host")).isEqualTo("proxy.hitchhiker.org:3128");
return null;
});
INIConfiguration ini = assertHgrc();
INISection proxy = ini.getSection("http_proxy");
assertThat(proxy).isNotNull();
assertThat(proxy.getParameter("host")).isEqualTo("awesome.hitchhiker.com:3128");
}
private void writeAuthentication() throws IOException {
INIConfiguration configuration = new INIConfiguration();
INISection auth = new INISection("auth");
auth.setParameter("a.prefix", "awesome.hitchhiker.com");
auth.setParameter("a.schemes", "ssh");
auth.setParameter("a.username", "dent");
auth.setParameter("a.password", "arthur123");
configuration.addSection(auth);
INIConfigurationWriter writer = new INIConfigurationWriter();
writer.write(configuration, hgrc.toFile());
}
private void writeProxyConfiguration() throws IOException {
INIConfiguration configuration = new INIConfiguration();
INISection proxy = new INISection("http_proxy");
proxy.setParameter("host", "awesome.hitchhiker.com:3128");
proxy.setParameter("user", "dent");
proxy.setParameter("passwd", "arthur123");
configuration.addSection(proxy);
INIConfigurationWriter writer = new INIConfigurationWriter();
writer.write(configuration, hgrc.toFile());
}
}