--web-bundle for v2 CLI: fix #1002 and allow overrides

src/bin/tectonic/compile.rs

@@ -36,11 +36,6 @@ pub struct CompileOptions {
     #[structopt(takes_value(true), parse(from_os_str), long, short, name = "file_path")]
     bundle: Option<PathBuf>,
 
-    /// Use this URL to find resource files instead of the default
-    #[structopt(takes_value(true), long, short, name = "url")]
-    // TODO add URL validation
-    web_bundle: Option<String>,
-
     /// Use only resource files cached locally
     #[structopt(short = "C", long)]
     only_cached: bool,
@@ -95,7 +90,12 @@ pub struct CompileOptions {
 }
 
 impl CompileOptions {
-    pub fn execute(self, config: PersistentConfig, status: &mut dyn StatusBackend) -> Result<i32> {
+    pub fn execute(
+        self,
+        config: PersistentConfig,
+        status: &mut dyn StatusBackend,
+        web_bundle: Option<String>,
+    ) -> Result<i32> {
         let unstable = UnstableOptions::from_unstable_args(self.unstable.into_iter());
 
         // Default to allowing insecure since it would be super duper annoying
@@ -193,7 +193,7 @@ impl CompileOptions {
         }
         if let Some(path) = self.bundle {
             sess_builder.bundle(config.make_local_file_provider(path, status)?);
-        } else if let Some(u) = self.web_bundle {
+        } else if let Some(u) = web_bundle {
             sess_builder.bundle(config.make_cached_url_provider(&u, only_cached, None, status)?);
         } else {
             sess_builder.bundle(config.default_bundle(only_cached, status)?);

src/bin/tectonic/main.rs

@@ -6,7 +6,6 @@ use std::{env, process, str::FromStr};
 use structopt::StructOpt;
 use tectonic_status_base::plain::PlainStatusBackend;
 
-use structopt::clap;
 use tectonic::{
     config::PersistentConfig,
     errors::SyncError,
@@ -40,18 +39,23 @@ mod v2cli {
 #[derive(Debug, StructOpt)]
 #[structopt(name = "Tectonic", about = "Process a (La)TeX document")]
 struct CliOptions {
-    /// Use experimental V2 interface (see `tectonic -X --help`); must be the first argument
+    /// Use experimental V2 interface (see `tectonic -X --help`)
     #[structopt(short = "X")]
     use_v2: bool,
 
     /// How much chatter to print when running
     #[structopt(long = "chatter", short, name = "level", default_value = "default", possible_values(&["default", "minimal"]))]
     chatter_level: String,
 
-    /// Enable/disable colorful log output.
+    /// Enable/disable colorful log output
     #[structopt(long = "color", name = "when", default_value = "auto", possible_values(&["always", "auto", "never"]))]
     cli_color: String,
 
+    /// Use this URL to find resource files instead of the default
+    #[structopt(takes_value(true), long, short, name = "url", overrides_with = "url")]
+    // TODO add URL validation
+    web_bundle: Option<String>,
+
     #[structopt(flatten)]
     compile: compile::CompileOptions,
 }
@@ -78,26 +82,34 @@ fn main() {
         unstable_opts::UnstableOptions::from_unstable_args(args.unstable.into_iter());
     }
 
-    // Migration to the "cargo-style" command-line interface. If the first
-    // argument is `-X`, or argv[0] contains `nextonic`, we activate the
+    // Migration to the "cargo-style" command-line interface. If the arguments
+    // list contains `-X`, or argv[0] contains `nextonic`, we activate the
     // alternative operation mode. Once this experimental mode is working OK,
     // we'll start printing a message telling people to prefer the `-X` option
     // and use `-X compile` for the "classic" ("rustc"-style, current)
     // interface. After that's been in place for a while, we'll make V2 mode the
     // default.
 
     let mut v2cli_enabled = false;
-    let mut v2cli_arg_idx = 1;
+    let mut v2cli_args = os_args[1..].to_vec(); // deep copy
 
     if !os_args.is_empty() && os_args[0].to_str().map(|s| s.contains("nextonic")) == Some(true) {
         v2cli_enabled = true;
-    } else if os_args.len() > 1 && os_args[1] == "-X" {
-        v2cli_enabled = true;
-        v2cli_arg_idx = 2;
+    } else if let Some(index) = v2cli_args
+        .to_vec()
+        .iter()
+        .position(|s| s.to_str().unwrap_or_default() == "-X")
+    {
+        // Try to parse as v1 cli first, and when that doesn't work,
+        // interpret it as v2 cli:
+        if CliOptions::from_args_safe().is_err() || CliOptions::from_args().use_v2 {
+            v2cli_enabled = true;
+            v2cli_args.remove(index);
+        }
     }
 
     if v2cli_enabled {
-        v2cli::v2_main(&os_args[v2cli_arg_idx..]);
+        v2cli::v2_main(&v2cli_args);
         return;
     }
 
@@ -154,20 +166,11 @@ fn main() {
         Box::new(PlainStatusBackend::new(chatter_level)) as Box<dyn StatusBackend>
     };
 
-    if args.use_v2 {
-        let err = clap::Error::with_description(
-            "-X option must be the first argument if given",
-            clap::ErrorKind::ArgumentConflict,
-        );
-        status.report_error(&err.into());
-        process::exit(1)
-    }
-
     // Now that we've got colorized output, pass off to the inner function ...
     // all so that we can print out the word "error:" in red. This code
     // parallels various bits of the `error_chain` crate.
 
-    if let Err(e) = args.compile.execute(config, &mut *status) {
+    if let Err(e) = args.compile.execute(config, &mut *status, args.web_bundle) {
         status.report_error(&SyncError::new(e).into());
         process::exit(1)
     }

src/bin/tectonic/v2cli.rs

@@ -62,6 +62,18 @@ struct V2CliOptions {
     )]
     cli_color: String,
 
+    /// Use this URL to find resource files instead of the default
+    #[structopt(
+        takes_value(true),
+        long,
+        short,
+        name = "url",
+        overrides_with = "url",
+        global(true)
+    )]
+    // TODO add URL validation
+    web_bundle: Option<String>,
+
     #[structopt(subcommand)]
     command: Commands,
 }
@@ -138,7 +150,7 @@ pub fn v2_main(effective_args: &[OsString]) {
 
     // Now that we've got colorized output, pass off to the inner function.
 
-    let code = match args.command.execute(config, &mut *status) {
+    let code = match args.command.execute(config, &mut *status, args.web_bundle) {
         Ok(c) => c,
         Err(e) => {
             status.report_error(&SyncError::new(e).into());
@@ -204,14 +216,19 @@ impl Commands {
         }
     }
 
-    fn execute(self, config: PersistentConfig, status: &mut dyn StatusBackend) -> Result<i32> {
+    fn execute(
+        self,
+        config: PersistentConfig,
+        status: &mut dyn StatusBackend,
+        web_bundle: Option<String>,
+    ) -> Result<i32> {
         match self {
-            Commands::Build(o) => o.execute(config, status),
+            Commands::Build(o) => o.execute(config, status, web_bundle),
             Commands::Bundle(o) => o.execute(config, status),
-            Commands::Compile(o) => o.execute(config, status),
+            Commands::Compile(o) => o.execute(config, status, web_bundle),
             Commands::Dump(o) => o.execute(config, status),
-            Commands::New(o) => o.execute(config, status),
-            Commands::Init(o) => o.execute(config, status),
+            Commands::New(o) => o.execute(config, status, web_bundle),
+            Commands::Init(o) => o.execute(config, status, web_bundle),
             Commands::Show(o) => o.execute(config, status),
             Commands::Watch(o) => o.execute(config, status),
             Commands::External(args) => do_external(args),
@@ -254,7 +271,18 @@ pub struct BuildCommand {
 impl BuildCommand {
     fn customize(&self, _cc: &mut CommandCustomizations) {}
 
-    fn execute(self, config: PersistentConfig, status: &mut dyn StatusBackend) -> Result<i32> {
+    fn execute(
+        self,
+        config: PersistentConfig,
+        status: &mut dyn StatusBackend,
+        web_bundle: Option<String>,
+    ) -> Result<i32> {
+        // `--web-bundle` is not actually used for `-X build`,
+        // so inform the user instead of ignoring silently.
+        if let Some(url) = web_bundle {
+            tt_note!(status, "--web-bundle {} ignored", &url);
+            tt_note!(status, "using workspace bundle configuration");
+        }
         let ws = Workspace::open_from_environment()?;
         let doc = ws.first_document();
 
@@ -681,7 +709,12 @@ pub struct NewCommand {
 impl NewCommand {
     fn customize(&self, _cc: &mut CommandCustomizations) {}
 
-    fn execute(self, config: PersistentConfig, status: &mut dyn StatusBackend) -> Result<i32> {
+    fn execute(
+        self,
+        config: PersistentConfig,
+        status: &mut dyn StatusBackend,
+        web_bundle: Option<String>,
+    ) -> Result<i32> {
         tt_note!(
             status,
             "creating new document in directory `{}`",
@@ -690,7 +723,7 @@ impl NewCommand {
 
         let wc = WorkspaceCreator::new(self.path);
         ctry!(
-            wc.create_defaulted(&config, status);
+            wc.create_defaulted(config, status, web_bundle);
             "failed to create the new Tectonic workspace"
         );
         Ok(0)
@@ -704,7 +737,12 @@ pub struct InitCommand {}
 impl InitCommand {
     fn customize(&self, _cc: &mut CommandCustomizations) {}
 
-    fn execute(self, config: PersistentConfig, status: &mut dyn StatusBackend) -> Result<i32> {
+    fn execute(
+        self,
+        config: PersistentConfig,
+        status: &mut dyn StatusBackend,
+        web_bundle: Option<String>,
+    ) -> Result<i32> {
         let path = env::current_dir()?;
         tt_note!(
             status,
@@ -714,7 +752,7 @@ impl InitCommand {
 
         let wc = WorkspaceCreator::new(path);
         ctry!(
-            wc.create_defaulted(&config, status);
+            wc.create_defaulted(config, status, web_bundle);
             "failed to create the new Tectonic workspace"
         );
         Ok(0)

src/config.rs

@@ -44,6 +44,25 @@ pub fn is_config_test_mode_activated() -> bool {
     CONFIG_TEST_MODE_ACTIVATED.load(Ordering::SeqCst)
 }
 
+pub fn is_test_bundle_wanted(web_bundle: Option<String>) -> bool {
+    if !is_config_test_mode_activated() {
+        return false;
+    }
+    match web_bundle {
+        None => true,
+        Some(x) if x.contains("test-bundle://") => true,
+        _ => false,
+    }
+}
+
+pub fn maybe_return_test_bundle(web_bundle: Option<String>) -> Result<Box<dyn Bundle>> {
+    if is_test_bundle_wanted(web_bundle) {
+        Ok(Box::<crate::test_util::TestBundle>::default())
+    } else {
+        Err(ErrorKind::Msg("not asking for the default test bundle".to_owned()).into())
+    }
+}
+
 #[cfg_attr(feature = "serde", derive(Deserialize, Serialize))]
 pub struct PersistentConfig {
     default_bundles: Vec<BundleInfo>,
@@ -122,6 +141,10 @@ impl PersistentConfig {
         custom_cache_root: Option<&Path>,
         status: &mut dyn StatusBackend,
     ) -> Result<Box<dyn Bundle>> {
+        if let Ok(test_bundle) = maybe_return_test_bundle(Some(url.to_owned())) {
+            return Ok(test_bundle);
+        }
+
         let mut cache = if let Some(root) = custom_cache_root {
             Cache::get_for_custom_directory(root)
         } else {
@@ -156,9 +179,8 @@ impl PersistentConfig {
     ) -> Result<Box<dyn Bundle>> {
         use std::io;
 
-        if CONFIG_TEST_MODE_ACTIVATED.load(Ordering::SeqCst) {
-            let bundle = crate::test_util::TestBundle::default();
-            return Ok(Box::new(bundle));
+        if let Ok(test_bundle) = maybe_return_test_bundle(None) {
+            return Ok(test_bundle);
         }
 
         if self.default_bundles.len() != 1 {
@@ -183,7 +205,7 @@ impl PersistentConfig {
     }
 
     pub fn format_cache_path(&self) -> Result<PathBuf> {
-        if CONFIG_TEST_MODE_ACTIVATED.load(Ordering::SeqCst) {
+        if is_config_test_mode_activated() {
             Ok(crate::test_util::test_path(&[]))
         } else {
             Ok(app_dirs::ensure_user_cache_dir("formats")?)

src/docmodel.rs

@@ -28,7 +28,7 @@ use crate::{
     driver::{OutputFormat, PassSetting, ProcessingSessionBuilder},
     errors::{ErrorKind, Result},
     status::StatusBackend,
-    test_util, tt_note,
+    tt_note,
     unstable_opts::UnstableOptions,
 };
 
@@ -111,9 +111,8 @@ impl DocumentExt for Document {
             }
         }
 
-        if config::is_config_test_mode_activated() {
-            let bundle = test_util::TestBundle::default();
-            Ok(Box::new(bundle))
+        if let Ok(test_bundle) = config::maybe_return_test_bundle(None) {
+            Ok(test_bundle)
         } else if let Ok(url) = Url::parse(&self.bundle_loc) {
             if url.scheme() != "file" {
                 let mut cache = Cache::get_user_default()?;
@@ -216,22 +215,25 @@ pub trait WorkspaceCreatorExt {
     /// for the main document.
     fn create_defaulted(
         self,
-        config: &config::PersistentConfig,
+        config: config::PersistentConfig,
         status: &mut dyn StatusBackend,
+        web_bundle: Option<String>,
     ) -> Result<Workspace>;
 }
 
 impl WorkspaceCreatorExt for WorkspaceCreator {
     fn create_defaulted(
         self,
-        config: &config::PersistentConfig,
+        config: config::PersistentConfig,
         status: &mut dyn StatusBackend,
+        web_bundle: Option<String>,
     ) -> Result<Workspace> {
-        let bundle_loc = if config::is_config_test_mode_activated() {
+        let bundle_loc = if config::is_test_bundle_wanted(web_bundle.clone()) {
             "test-bundle://".to_owned()
         } else {
+            let unresolved_loc = web_bundle.unwrap_or(config.default_bundle_loc().to_owned());
             let mut gub = DefaultBackend::default();
-            gub.resolve_url(config.default_bundle_loc(), status)?
+            gub.resolve_url(&unresolved_loc, status)?
         };
 
         Ok(self.create(bundle_loc)?)

tests/executable.rs

@@ -636,6 +636,141 @@ fn stdin_content() {
     success_or_panic(&output);
 }
 
+/// Test various web bundle overrides for the v1 CLI & `-X compile`
+#[test]
+fn web_bundle_overrides() {
+    let filename = "subdirectory/content/1.tex";
+    let fmt_arg: &str = &get_plain_format_arg();
+    let tempdir = setup_and_copy_files(&[filename]);
+    let temppath = tempdir.path().to_owned();
+
+    let arg_bad_bundle = ["--web-bundle", "bad-bundle"];
+    let arg_good_bundle = ["--web-bundle", "test-bundle://"];
+
+    // test with a bad bundle
+    let output = run_tectonic(
+        &temppath,
+        &[&arg_bad_bundle[..], &[fmt_arg, filename]].concat(),
+    );
+    error_or_panic(&output);
+
+    // test with a good bundle (override)
+    let mut valid_args: Vec<Vec<&str>> = vec![
+        // different positions
+        [&arg_good_bundle[..], &[fmt_arg, filename]].concat(),
+        [&[fmt_arg], &arg_good_bundle[..], &[filename]].concat(),
+        [&[fmt_arg], &[filename], &arg_good_bundle[..]].concat(),
+        // overriding vendor presets
+        [
+            &arg_bad_bundle[..],
+            &arg_good_bundle[..],
+            &[fmt_arg],
+            &[filename],
+        ]
+        .concat(),
+        // stress test
+        [
+            &arg_bad_bundle[..],
+            &arg_bad_bundle[..],
+            &[fmt_arg],
+            &arg_bad_bundle[..],
+            &arg_bad_bundle[..],
+            &[filename],
+            &arg_bad_bundle[..],
+            &arg_good_bundle[..],
+        ]
+        .concat(),
+    ];
+
+    // test `-X compile`
+    #[cfg(feature = "serialization")]
+    valid_args.push(
+        [
+            &arg_bad_bundle[..],
+            &arg_bad_bundle[..],
+            &["-X"],
+            &arg_bad_bundle[..],
+            &["compile"],
+            &arg_bad_bundle[..],
+            &[fmt_arg],
+            &arg_bad_bundle[..],
+            &[filename],
+            &arg_bad_bundle[..],
+            &arg_good_bundle[..],
+        ]
+        .concat(),
+    );
+
+    for args in valid_args {
+        let output = run_tectonic(&temppath, &args);
+        success_or_panic(&output);
+    }
+}
+
+/// Test various web bundle overrides for the v2 CLI
+#[cfg(feature = "serialization")]
+#[test]
+fn v2_bundle_overrides() {
+    let arg_bad_bundle = ["--web-bundle", "bad-bundle"];
+    let arg_good_bundle = ["--web-bundle", "test-bundle://"];
+
+    // test `-X command`
+    for command in ["new", "init"] {
+        // test with a bad bundle
+        let tempdir = setup_and_copy_files(&[]);
+        let temppath = tempdir.path().to_owned();
+        let output = run_tectonic(&temppath, &[&arg_bad_bundle[..], &["-X", command]].concat());
+        error_or_panic(&output);
+
+        // test with a good bundle (override)
+        let valid_args: Vec<Vec<&str>> = vec![
+            // different positions
+            [&arg_good_bundle[..], &["-X", command]].concat(),
+            [&["-X"], &arg_good_bundle[..], &[command]].concat(),
+            [&["-X", command], &arg_good_bundle[..]].concat(),
+            // overriding vendor presets
+            [&arg_bad_bundle[..], &arg_good_bundle[..], &["-X", command]].concat(),
+            [
+                &arg_bad_bundle[..],
+                &["-X"],
+                &arg_good_bundle[..],
+                &[command],
+            ]
+            .concat(),
+            [&arg_bad_bundle[..], &["-X", command], &arg_good_bundle[..]].concat(),
+            // stress test
+            [
+                &arg_bad_bundle[..],
+                &arg_bad_bundle[..],
+                &["-X"],
+                &arg_bad_bundle[..],
+                &arg_bad_bundle[..],
+                &[command],
+                &arg_bad_bundle[..],
+                &arg_good_bundle[..],
+            ]
+            .concat(),
+        ];
+
+        for args in valid_args {
+            let tempdir = setup_and_copy_files(&[]);
+            let temppath = tempdir.path().to_owned();
+            let output = run_tectonic(&temppath, &args);
+            success_or_panic(&output);
+        }
+    }
+
+    // test `-X build`
+    let (_tempdir, temppath) = setup_v2();
+
+    // `--web-bundle` is ignored
+    let output = run_tectonic(
+        &temppath,
+        &[&arg_bad_bundle[..], &["-X"], &["build"]].concat(),
+    );
+    success_or_panic(&output);
+}
+
 #[cfg(feature = "serialization")]
 #[test]
 fn v2_build_basic() {
@@ -926,21 +1061,6 @@ fn extra_search_paths() {
     error_or_panic(&output);
 }
 
-/// -X in non-initial position fails
-#[test]
-fn bad_v2_position() {
-    let output = run_tectonic(&PathBuf::from("."), &["-", "-X"]);
-    error_or_panic(&output);
-}
-
-#[cfg(feature = "serialization")]
-#[test]
-fn bad_v2_position_build() {
-    let (_tempdir, temppath) = setup_v2();
-    let output = run_tectonic(&temppath, &["build", "-X"]);
-    error_or_panic(&output);
-}
-
 /// Ensures that watch command succeeds, and when a file is changed while running it rebuilds
 /// periodically
 #[cfg(all(feature = "serialization", not(target_arch = "mips")))]