-
-
Notifications
You must be signed in to change notification settings - Fork 51
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
Make source archive download command configurable #1815
Make source archive download command configurable #1815
Conversation
Ready for review @mosteo. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A nice set of improvements, I identified just a couple of minor changes.
src/alire/alire-settings-edit.adb
Outdated
function Valid_Builtin_Check (Lvl : Level) return CLIC.Config.Check_Import | ||
is (case Lvl is | ||
when Global => Valid_Global_Builtin'Access, | ||
when others => Valid_Local_Builtin'Access); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather have here the explicit case instead of others
.
assert_match(r"arg1 abproject1\.gprab arg3", p.out) | ||
|
||
# Verify that an `editor.cmd` value in a crate's local `settings.toml` is | ||
# ignored with a warning (otherwise this would offer an inconspicouous vector |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/inconspicouous/inconspicuous/
testsuite/drivers/alr.py
Outdated
def unset_setting(key: str, local: bool = False): | ||
""" | ||
Unset a key with `alr settings` | ||
|
||
Sets the value globally unless `local` is `True`. | ||
""" | ||
if local: | ||
run_alr("settings", "--unset", key) | ||
else: | ||
run_alr("settings", "--global", "--unset", key) | ||
|
||
def set_setting(key: str, value: str, local: bool = False): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency with other alr_blah
, I'd rename those to alr_settings_set
and alr_settings_unset
.
At the moment, the biggest obstacle to using Alire for private development is that source archives cannot be fetched from servers which require user authentication (this is already possible for private Git repositories, since the
git
command implements all authentication schemes commonly used for Git repositories).Unfortunately, there is a much wider variety of commonly used schemes and protocols for authenticated file downloads than for Git repos, so it does not seem feasible to support all of them natively. This PR therefore makes the command used for downloads (globally) configurable with the
alr settings
keyorigins.archive.download_cmd
, without changing the default behaviour.This PR also:
editor.cmd
setting being read from the local crate'ssettings.toml
(this seemed to me unexpected behaviour, and possibly something of a security vulnerability)distribution.override
settingfail_on_error
in thespellcheck
workflowPR creation checklist
doc/user-changes.md
has been updated.