diff --git a/openc3/lib/openc3/models/plugin_model.rb b/openc3/lib/openc3/models/plugin_model.rb index ec816e88e0..5a5d4b45d8 100644 --- a/openc3/lib/openc3/models/plugin_model.rb +++ b/openc3/lib/openc3/models/plugin_model.rb @@ -598,14 +598,12 @@ def self.gem_names def self.resolve_pypi_url(scope:) pypi_url = nil begin - pypi_url = get_setting('pypi_url', scope: scope) - pypi_url += '/simple' if pypi_url + pypi_url = PypiUrl.index_url(get_setting('pypi_url', scope: scope)) rescue => e Logger.error("Failed to retrieve pypi_url: #{e.formatted}") ensure if pypi_url.nil? - pypi_url = ENV.fetch('PYPI_URL', nil) - pypi_url += '/simple' if pypi_url + pypi_url = PypiUrl.index_url(ENV.fetch('PYPI_URL', nil)) pypi_url ||= PypiUrl::DEFAULT end end diff --git a/openc3/lib/openc3/models/python_package_model.rb b/openc3/lib/openc3/models/python_package_model.rb index c7f96738f3..d91e055240 100644 --- a/openc3/lib/openc3/models/python_package_model.rb +++ b/openc3/lib/openc3/models/python_package_model.rb @@ -235,19 +235,13 @@ def self.install(name_or_path, scope:, plugin: nil) end package_filename = File.basename(package_file_path) begin - pypi_url = get_setting('pypi_url', scope: scope) - if pypi_url - pypi_url += '/simple' - end + pypi_url = PypiUrl.index_url(get_setting('pypi_url', scope: scope)) rescue => e Logger.error("Failed to retrieve pypi_url: #{e.formatted}") ensure if pypi_url.nil? # If Redis isn't running try the ENV, then simply pypi.org/simple - pypi_url = ENV.fetch('PYPI_URL', nil) - if pypi_url - pypi_url += '/simple' - end + pypi_url = PypiUrl.index_url(ENV.fetch('PYPI_URL', nil)) pypi_url ||= PypiUrl::DEFAULT end end diff --git a/openc3/lib/openc3/utilities/pypi_url.rb b/openc3/lib/openc3/utilities/pypi_url.rb index a9db1d38cc..cc951a42d9 100644 --- a/openc3/lib/openc3/utilities/pypi_url.rb +++ b/openc3/lib/openc3/utilities/pypi_url.rb @@ -18,6 +18,18 @@ module OpenC3 class PypiUrl DEFAULT = 'https://pypi.org/simple' + # Turn a configured PyPI base URL (the pypi_url setting or PYPI_URL) into + # its simple index URL. Trailing slashes are dropped first, matching the + # ${PYPI_URL%/}/simple the image builds use, so https://mirror/ does not + # give https://mirror//simple and https://pypi.org/ still equals DEFAULT. + # + # @param base_url [String, nil] the configured base url + # @return [String, nil] the simple index url, or nil when base_url is nil + def self.index_url(base_url) + return nil if base_url.nil? + "#{base_url.sub(%r{/+\z}, '')}/simple" + end + # Validate that a resolved pypi_url is an http(s) URL before it is handed to # pip. The value can come from a user-writable setting or ENV, so a malformed # or non-http value is rejected and replaced with the default rather than diff --git a/openc3/spec/models/plugin_model_spec.rb b/openc3/spec/models/plugin_model_spec.rb index a5eed5925f..779d397820 100644 --- a/openc3/spec/models/plugin_model_spec.rb +++ b/openc3/spec/models/plugin_model_spec.rb @@ -729,6 +729,24 @@ def install_plugin expect(PluginModel.resolve_pypi_url(scope: "DEFAULT")).to eql "https://custom.pypi.example.com/simple" end + it "drops a trailing slash from the pypi_url setting" do + allow(PluginModel).to receive(:get_setting).with('pypi_url', scope: "DEFAULT").and_return("https://custom.pypi.example.com/") + expect(PluginModel.resolve_pypi_url(scope: "DEFAULT")).to eql "https://custom.pypi.example.com/simple" + end + + it "treats the public index with a trailing slash as the default" do + allow(PluginModel).to receive(:get_setting).with('pypi_url', scope: "DEFAULT").and_return("https://pypi.org/") + pypi_url = PluginModel.resolve_pypi_url(scope: "DEFAULT") + expect(pypi_url).to eql PypiUrl::DEFAULT + expect(PluginModel.build_pypi_args(pypi_url)).to_not include("--no-config") + end + + it "drops a trailing slash from ENV PYPI_URL" do + allow(PluginModel).to receive(:get_setting).with('pypi_url', scope: "DEFAULT").and_return(nil) + allow(ENV).to receive(:fetch).with('PYPI_URL', nil).and_return("https://env.pypi.example.com/") + expect(PluginModel.resolve_pypi_url(scope: "DEFAULT")).to eql "https://env.pypi.example.com/simple" + end + it "falls back to ENV PYPI_URL when the setting is nil" do allow(PluginModel).to receive(:get_setting).with('pypi_url', scope: "DEFAULT").and_return(nil) allow(ENV).to receive(:fetch).with('PYPI_URL', nil).and_return("https://env.pypi.example.com") diff --git a/openc3/spec/models/python_package_model_spec.rb b/openc3/spec/models/python_package_model_spec.rb index 6df1dbf71e..dc3b3e233f 100644 --- a/openc3/spec/models/python_package_model_spec.rb +++ b/openc3/spec/models/python_package_model_spec.rb @@ -434,6 +434,13 @@ def install_spawn(**kwargs) expect(cmd).to include("https://custom.pypi.example.com/simple") end + it "drops a trailing slash from the pypi_url setting" do + allow(PythonPackageModel).to receive(:get_setting).with('pypi_url', scope: "DEFAULT").and_return("https://custom.pypi.example.com/") + + cmd, _kw = install_spawn + expect(cmd).to include("https://custom.pypi.example.com/simple") + end + it "falls back to ENV PYPI_URL when get_setting raises" do allow(ENV).to receive(:fetch).with('PYPI_URL', nil).and_return("https://env.pypi.example.com") allow(PythonPackageModel).to receive(:get_setting).and_raise("no redis") diff --git a/openc3/spec/utilities/pypi_url_spec.rb b/openc3/spec/utilities/pypi_url_spec.rb index 0365724eb0..e39eed72fb 100644 --- a/openc3/spec/utilities/pypi_url_spec.rb +++ b/openc3/spec/utilities/pypi_url_spec.rb @@ -16,6 +16,25 @@ module OpenC3 describe PypiUrl do + describe ".index_url" do + it "appends /simple to a base url" do + expect(PypiUrl.index_url("https://mirror.example.com")).to eql "https://mirror.example.com/simple" + end + + it "drops trailing slashes before appending /simple" do + expect(PypiUrl.index_url("https://mirror.example.com/")).to eql "https://mirror.example.com/simple" + expect(PypiUrl.index_url("https://mirror.example.com/pypi//")).to eql "https://mirror.example.com/pypi/simple" + end + + it "maps the public index with a trailing slash to DEFAULT" do + expect(PypiUrl.index_url("https://pypi.org/")).to eql PypiUrl::DEFAULT + end + + it "returns nil for nil" do + expect(PypiUrl.index_url(nil)).to be_nil + end + end + describe ".validate" do context "with a valid http(s) url" do it "returns https urls unchanged" do