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

scripts/download.pl: pass aria2 config in ENV only #10874

Merged
merged 1 commit into from Oct 20, 2022
Merged

scripts/download.pl: pass aria2 config in ENV only #10874

merged 1 commit into from Oct 20, 2022

Conversation

arenekosreal
Copy link
Contributor

The aria2c command tries to load config from
${XDG_CONFIG_HOME:-${HOME}/.config}/aria2/aria2.conf by default, which may result unexpected behavior.

As a replacement, people can use environment variable ARIA2C_OPTIONS to custom arguments passed to aria2c like curl and wget below.

Signed-off-by: Zhang Hua zhanghuadedn@gmail.com

@github-actions github-actions bot added the build/scripts/tools pull request/issues for build, scripts and tools related changes label Oct 2, 2022
@aparcar
Copy link
Member

aparcar commented Oct 2, 2022

Is ARIA2C_OPTIONS something official or did you come up with it? Also why not use the local configuration? Isn't that usually the point of having a local configuration?

@arenekosreal
Copy link
Contributor Author

arenekosreal commented Oct 2, 2022

@aparcar
No, I just follow the name pattern of curl's CURL_OPTIONS and wget's WGET_OPTIONS to create an environment variable named ARIA2C_OPTIONS.

As far as I know, aria2c will only read environment variable for proxy, like HTTPS_PROXY,HTTP_PROXY and so on. Those environment variables' value can also be override by passing command-line arguments like --https-proxy and --http-proxy

Using the local configuration may be conflict with existing aria2c instance. Take me as an example, I am using that config file for an existing aria2c daemon which will listen a RPC port, if I run the build workflow, the aria2c called by build system will also try to listen that RPC port, of course it will be failed to listen the RPC port because there has already been one listening, the build workflow will also be failed to run due to this.

People have to use a huge number of command-line options if they don't use a config file. As aria2c has an option to custom where the config file is, I must say there may not be so many people use default config path.

@Ansuel
Copy link
Member

Ansuel commented Oct 5, 2022

@zhanghua000 we are sorting some problem with download tool configurable but this LGTM. Will merge when we sort other stuff... while we wait can you fix the title to something more understandable and fix the tag?

Something like scripts/download.pl: permit to pass aria2c config with ENV variables

@Ansuel Ansuel self-assigned this Oct 5, 2022
@arenekosreal arenekosreal changed the title build: scripts/download.pl: call aria2c cleanly scripts/download.pl: pass aria2 config in ENV only Oct 6, 2022
@arenekosreal
Copy link
Contributor Author

@Ansuel
I have made my branch rebased and made the tag updated, I think this should meet the requirement now.

@github-actions github-actions bot added core packages pull request/issue for core (in-tree) packages GitHub/CI pull requests/issues for GitHub, CI and related stuff kernel pull request/issue with Linux kernel related changes target/ipq40xx pull request/issue for ipq40xx target target/lantiq pull request/issue for lantiq target target/mpc85xx pull request/issue for mpc85xx target target/qoriq pull request/issue for qoriq target target/ramips pull request/issue for ramips target target/realtek pull request/issue for realtek target target/rockchip pull request/issue for rockchip target target/uml pull request/issue for uml target toolchain pull request/issue with toolchain related changes target/ath79 pull request/issue for ath79 target and removed toolchain pull request/issue with toolchain related changes GitHub/CI pull requests/issues for GitHub, CI and related stuff core packages pull request/issue for core (in-tree) packages target/realtek pull request/issue for realtek target target/qoriq pull request/issue for qoriq target target/mpc85xx pull request/issue for mpc85xx target target/rockchip pull request/issue for rockchip target target/uml pull request/issue for uml target target/ramips pull request/issue for ramips target target/lantiq pull request/issue for lantiq target labels Oct 6, 2022
@github-actions github-actions bot removed target/ath79 pull request/issue for ath79 target target/ipq40xx pull request/issue for ipq40xx target kernel pull request/issue with Linux kernel related changes labels Oct 10, 2022
@Ansuel
Copy link
Member

Ansuel commented Oct 19, 2022

@zhanghua000 can you rebase this with recent changes?

@arenekosreal
Copy link
Contributor Author

@Ansuel I have rebased my commit with recent changes.

The aria2c command tries to load config from
${XDG_CONFIG_HOME:-${HOME}/.config}/aria2/aria2.conf by default,
which may result unexpected behavior.

As a replacement, people can use environment variable ARIA2C_OPTIONS
to custom arguments passed to aria2c like curl and wget below.
Including --conf-path=/path/to/config.conf in ARIA2C_OPTIONS can
also set a custom config file path easily if needed.

Signed-off-by: Zhang Hua <zhanghuadedn@gmail.com>
@openwrt-bot openwrt-bot merged commit a53f29b into openwrt:master Oct 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build/scripts/tools pull request/issues for build, scripts and tools related changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants