diff --git a/js/index.d.ts b/js/index.d.ts index 61ddef9416..00af84e98b 100644 --- a/js/index.d.ts +++ b/js/index.d.ts @@ -49,12 +49,19 @@ declare module '@sentry/cli' { silent?: boolean; } + /** + * Custom upload-sourcemaps options for a particular `include` path. In this + * case `paths` takes the place of `include` in the options so as to make it + * clear that this is not recursive. + */ + export type SourceMapsPathDescriptor = Omit & { paths: string[] } + export interface SentryCliUploadSourceMapsOptions { /** * One or more paths that Sentry CLI should scan recursively for sources. * It will upload all .map files and match associated .js files. */ - include: string | string[]; + include: Array; /** * One or more paths to ignore during upload. Overrides entries in ignoreFile file. */ diff --git a/js/releases/__tests__/index.test.js b/js/releases/__tests__/index.test.js index acd660b0bb..62a80c2cbd 100644 --- a/js/releases/__tests__/index.test.js +++ b/js/releases/__tests__/index.test.js @@ -94,6 +94,73 @@ describe('SentryCli releases', () => { { silent: false } ); }); + + test('handles multiple include entries', async () => { + expect.assertions(3); + + const paths = ['path', 'other-path']; + await cli.releases.uploadSourceMaps('my-version', { include: paths }); + + expect(mockExecute).toHaveBeenCalledTimes(2); + paths.forEach(path => + expect(mockExecute).toHaveBeenCalledWith( + [ + 'releases', + 'files', + 'my-version', + 'upload-sourcemaps', + path, + '--ignore', + 'node_modules', + ], + true, + false, + undefined, + { silent: false } + ) + ); + }); + + test('handles object-type include entries', async () => { + expect.assertions(3); + + const paths = [{ paths: ['some-path'], ignore: ['not-me'] }, 'other-path']; + await cli.releases.uploadSourceMaps('my-version', { include: paths }); + + expect(mockExecute).toHaveBeenCalledTimes(2); + + expect(mockExecute).toHaveBeenCalledWith( + [ + 'releases', + 'files', + 'my-version', + 'upload-sourcemaps', + 'some-path', + '--ignore', + 'not-me', // note how this has been overridden + ], + true, + false, + undefined, + { silent: false } + ); + + expect(mockExecute).toHaveBeenCalledWith( + [ + 'releases', + 'files', + 'my-version', + 'upload-sourcemaps', + 'other-path', + '--ignore', + 'node_modules', + ], + true, + false, + undefined, + { silent: false } + ); + }); }); }); }); diff --git a/js/releases/index.js b/js/releases/index.js index f4a6d35e35..983656508a 100644 --- a/js/releases/index.js +++ b/js/releases/index.js @@ -160,23 +160,58 @@ class Releases { * @memberof SentryReleases */ uploadSourceMaps(release, options) { - if (!options || !options.include) { - throw new Error('options.include must be a vaild path(s)'); + if (!options || !options.include || !Array.isArray(options.include)) { + throw new Error( + '`options.include` must be a vaild array of paths and/or path descriptor objects.' + ); } - const uploads = options.include.map(sourcemapPath => { - const newOptions = { ...options }; + // Each entry in the `include` array will map to an array of promises, which + // will in turn contain one promise per literal path value. Thus `uploads` + // will be an array of Promise arrays, which we'll flatten later. + const uploads = options.include.map(includeEntry => { + let pathOptions; + let uploadPaths; + + if (typeof includeEntry === 'object') { + pathOptions = includeEntry; + uploadPaths = includeEntry.paths; + + if (!Array.isArray(uploadPaths)) { + throw new Error( + `Path descriptor objects in \`options.include\` must contain a \`paths\` array. Got ${includeEntry}.` + ); + } + } + // `includeEntry` should be a string, which we can wrap in an array to + // match the `paths` property in the descriptor object type + else { + pathOptions = {}; + uploadPaths = [includeEntry]; + } + + const newOptions = { ...options, ...pathOptions }; if (!newOptions.ignoreFile && !newOptions.ignore) { newOptions.ignore = DEFAULT_IGNORE; } + // args which apply to the entire `include` entry (everything besides the path) const args = ['releases'] .concat(helper.getProjectFlagsFromOptions(options)) - .concat(['files', release, 'upload-sourcemaps', sourcemapPath]); - return this.execute(helper.prepareCommand(args, SOURCEMAPS_SCHEMA, newOptions), true); + .concat(['files', release, 'upload-sourcemaps']); + + return uploadPaths.map(path => + // `execute()` is async and thus we're returning a promise here + this.execute(helper.prepareCommand([...args, path], SOURCEMAPS_SCHEMA, newOptions), true) + ); }); - return Promise.all(uploads); + // `uploads` is an array of Promise arrays, which needs to be flattened + // before being passed to `Promise.all()`. (`Array.flat()` doesn't exist in + // Node < 11; this polyfill takes advantage of the fact that `concat()` is + // willing to accept an arbitrary number of items to add to and/or iterables + // to unpack into the given array.) + return Promise.all([].concat(...uploads)); } /** diff --git a/src/api.rs b/src/api.rs index 1772d1efb9..f6b1e8c5ec 100644 --- a/src/api.rs +++ b/src/api.rs @@ -451,7 +451,7 @@ impl Api { /// Convenience method that downloads a file into the given file object. pub fn download(&self, url: &str, dst: &mut File) -> ApiResult { - self.request(Method::Get, &url)? + self.request(Method::Get, url)? .follow_location(true)? .send_into(dst) } @@ -459,7 +459,7 @@ impl Api { /// Convenience method that downloads a file into the given file object /// and show a progress bar pub fn download_with_progress(&self, url: &str, dst: &mut File) -> ApiResult { - self.request(Method::Get, &url)? + self.request(Method::Get, url)? .follow_location(true)? .progress_bar_mode(ProgressBarMode::Response)? .send_into(dst) @@ -470,7 +470,7 @@ impl Api { pub fn wait_until_available(&self, url: &str, duration: Duration) -> ApiResult { let started = Utc::now(); loop { - match self.request(Method::Get, &url)?.send() { + match self.request(Method::Get, url)?.send() { Ok(_) => return Ok(true), Err(err) => { if err.kind() != ApiErrorKind::RequestFailed { @@ -1304,7 +1304,7 @@ impl Api { checkin: &CreateMonitorCheckIn, ) -> ApiResult { let path = &format!("/monitors/{}/checkins/", PathArg(monitor),); - let resp = self.post(&path, checkin)?; + let resp = self.post(path, checkin)?; if resp.status() == 404 { return Err(ApiErrorKind::ResourceNotFound.into()); } @@ -1323,7 +1323,7 @@ impl Api { PathArg(monitor), PathArg(checkin_id), ); - let resp = self.put(&path, checkin)?; + let resp = self.put(path, checkin)?; if resp.status() == 404 { return Err(ApiErrorKind::ResourceNotFound.into()); } @@ -1557,7 +1557,7 @@ impl ApiRequest { Method::Delete => handle.custom_request("DELETE")?, } - handle.url(&url)?; + handle.url(url)?; let request = ApiRequest { handle, diff --git a/src/commands/difutil_find.rs b/src/commands/difutil_find.rs index d61f2f54b2..89a85ea0ac 100644 --- a/src/commands/difutil_find.rs +++ b/src/commands/difutil_find.rs @@ -282,7 +282,7 @@ fn find_ids( eprintln!(); eprintln!("missing debug information files:"); for id in &remaining { - eprintln!(" {} ({})", id, id_hint(&id),); + eprintln!(" {} ({})", id, id_hint(id),); } } } diff --git a/src/commands/login.rs b/src/commands/login.rs index 06ff4b9b75..dd607bbbc4 100644 --- a/src/commands/login.rs +++ b/src/commands/login.rs @@ -33,7 +33,7 @@ pub fn execute(matches: &ArgMatches<'_>) -> Result<(), Error> { println!(); println!( "Sentry server: {}", - Url::parse(&config.get_base_url()?)? + Url::parse(config.get_base_url()?)? .host_str() .unwrap_or("") ); diff --git a/src/commands/releases.rs b/src/commands/releases.rs index 20b6e26ae6..29bec4ee0c 100644 --- a/src/commands/releases.rs +++ b/src/commands/releases.rs @@ -540,7 +540,7 @@ fn execute_set_commits<'a>( // make sure the release exists if projects are given if let Ok(projects) = ctx.get_projects(matches) { ctx.api.new_release( - &org, + org, &NewRelease { version: version.into(), projects, @@ -570,7 +570,7 @@ fn execute_set_commits<'a>( } table.print(); } - ctx.api.set_release_refs(&org, version, heads)?; + ctx.api.set_release_refs(org, version, heads)?; } else { let default_count = matches .value_of("initial-depth") @@ -739,7 +739,7 @@ fn execute_info<'a>(ctx: &ReleaseContext<'_>, matches: &ArgMatches<'a>) -> Resul let version = matches.value_of("version").unwrap(); let org = ctx.get_org()?; let project = ctx.get_project_default().ok(); - let release = ctx.api.get_release(org, project.as_deref(), &version)?; + let release = ctx.api.get_release(org, project.as_deref(), version)?; // quiet mode just exists if matches.is_present("quiet") { @@ -896,7 +896,7 @@ fn execute_files_upload<'a>( let ctx = &UploadContext { org, project: project.as_deref(), - release: &version, + release: version, dist, wait: matches.is_present("wait"), }; @@ -916,9 +916,9 @@ fn execute_files_upload<'a>( if let Some(artifact) = ctx.api.upload_release_file( org, project.as_deref(), - &version, + version, &FileContents::FromPath(path), - &name, + name, dist, Some(&headers[..]), ProgressBarMode::Request, @@ -1096,7 +1096,7 @@ fn execute_files_upload_sourcemaps<'a>( // make sure the release exists let release = ctx.api.new_release( - &org, + org, &NewRelease { version: version.into(), projects: ctx.get_projects(matches)?, diff --git a/src/commands/uninstall.rs b/src/commands/uninstall.rs index ac6e21a37b..406f3127cf 100644 --- a/src/commands/uninstall.rs +++ b/src/commands/uninstall.rs @@ -50,11 +50,11 @@ pub fn execute(matches: &ArgMatches<'_>) -> Result<(), Error> { return Err(QuietExit(1).into()); } - if !matches.is_present("confirm") { - if !prompt_to_continue("Do you really want to uninstall sentry-cli?")? { - println!("Aborted!"); - return Ok(()); - } + if !matches.is_present("confirm") + && !prompt_to_continue("Do you really want to uninstall sentry-cli?")? + { + println!("Aborted!"); + return Ok(()); } if !is_writable(&exe) { diff --git a/src/utils/dif_upload.rs b/src/utils/dif_upload.rs index 33b203eeda..4c2c1cccbe 100644 --- a/src/utils/dif_upload.rs +++ b/src/utils/dif_upload.rs @@ -204,8 +204,8 @@ impl<'data> DifMatch<'data> { pub fn data(&self) -> &[u8] { match self.dif.get() { ParsedDif::Object(ref obj) => obj.data(), - ParsedDif::BcSymbolMap(_) => &self.dif.owner(), - ParsedDif::UuidMap(_) => &self.dif.owner(), + ParsedDif::BcSymbolMap(_) => self.dif.owner(), + ParsedDif::UuidMap(_) => self.dif.owner(), } } @@ -896,7 +896,7 @@ fn resolve_hidden_symbols<'a>(dif: DifMatch<'a>, symbol_map: &Path) -> Result").dim(), style(batch.len()).yellow() ); - let archive = create_batch_archive(&batch)?; + let archive = create_batch_archive(batch)?; println!("{} Uploading debug symbol files", style(">").dim()); dsyms.extend(api.upload_dif_archive(&options.org, &options.project, archive.path())?); @@ -1937,7 +1937,7 @@ impl DifUpload { } // Skip if this DIF does not have features we want. - if !self.valid_features(&dif) { + if !self.valid_features(dif) { debug!("skipping {} because of features", dif.name); return false; } diff --git a/src/utils/file_search.rs b/src/utils/file_search.rs index 13caa67c08..827a8f379c 100644 --- a/src/utils/file_search.rs +++ b/src/utils/file_search.rs @@ -129,7 +129,7 @@ impl ReleaseFileSearch { if !&self.ignores.is_empty() { let mut override_builder = OverrideBuilder::new(&self.path); for ignore in &self.ignores { - override_builder.add(&ignore)?; + override_builder.add(ignore)?; } builder.overrides(override_builder.build()?); } diff --git a/src/utils/file_upload.rs b/src/utils/file_upload.rs index 484eb985c3..f619dc17e8 100644 --- a/src/utils/file_upload.rs +++ b/src/utils/file_upload.rs @@ -173,13 +173,8 @@ fn upload_files_parallel( if let Some(old_id) = release_files.get(&(context.dist.map(|x| x.into()), file.url.clone())) { - api.delete_release_file( - context.org, - context.project, - &context.release, - &old_id, - ) - .ok(); + api.delete_release_file(context.org, context.project, context.release, old_id) + .ok(); } api.upload_release_file( diff --git a/src/utils/sourcemaps.rs b/src/utils/sourcemaps.rs index 8dbc40f053..acbd66e853 100644 --- a/src/utils/sourcemaps.rs +++ b/src/utils/sourcemaps.rs @@ -145,7 +145,7 @@ fn guess_sourcemap_reference(sourcemaps: &HashSet, min_url: &str) -> Res let mut parts: Vec<_> = ext.split('.').collect(); if parts.len() > 1 { let parts_len = parts.len(); - parts[parts_len - 1] = &map_ext; + parts[parts_len - 1] = map_ext; let new_ext = parts.join("."); if sourcemaps.contains(&unsplit_url(path, basename, Some(&new_ext))) { return Ok(unsplit_url(None, basename, Some(&new_ext))); @@ -389,12 +389,12 @@ impl SourceMapProcessor { self.sources.remove(&sourcemap_url); let ram_bundle_iter = - sourcemap::ram_bundle::split_ram_bundle(&ram_bundle, &sourcemap_index).unwrap(); + sourcemap::ram_bundle::split_ram_bundle(ram_bundle, &sourcemap_index).unwrap(); for result in ram_bundle_iter { let (name, sourceview, sourcemap) = result?; debug!("Inserting source for {}", name); - let source_url = join_url(&bundle_source_url, &name)?; + let source_url = join_url(bundle_source_url, &name)?; self.sources.insert( source_url.clone(), ReleaseFile { diff --git a/src/utils/system.rs b/src/utils/system.rs index d6fe201799..4bf18515b2 100644 --- a/src/utils/system.rs +++ b/src/utils/system.rs @@ -110,7 +110,7 @@ pub fn expand_vars String>(s: &str, f: F) -> Cow<'_, str> { /// Helper that renders an error to stderr. pub fn print_error(err: &Error) { - if let Some(ref clap_err) = err.downcast_ref::() { + if let Some(clap_err) = err.downcast_ref::() { clap_err.exit(); } diff --git a/src/utils/vcs.rs b/src/utils/vcs.rs index db44c65c07..8c765a02d4 100644 --- a/src/utils/vcs.rs +++ b/src/utils/vcs.rs @@ -367,7 +367,7 @@ fn find_matching_revs( let rev = if let Some(rev) = find_matching_rev( spec.reference(), - &spec, + spec, repos, disable_discovery, remote_name.clone(), @@ -378,7 +378,7 @@ fn find_matching_revs( }; let prev_rev = if let Some(rev) = spec.prev_reference() { - if let Some(rv) = find_matching_rev(rev, &spec, repos, disable_discovery, remote_name)? { + if let Some(rv) = find_matching_rev(rev, spec, repos, disable_discovery, remote_name)? { Some(rv) } else { return Err(error(rev, &spec.repo)); @@ -410,7 +410,7 @@ pub fn find_heads( if let Some(specs) = specs { for spec in &specs { let (prev_rev, rev) = - find_matching_revs(&spec, repos, specs.len() == 1, remote_name.clone())?; + find_matching_revs(spec, repos, specs.len() == 1, remote_name.clone())?; rv.push(Ref { repo: spec.repo.clone(), rev, diff --git a/src/utils/xcode.rs b/src/utils/xcode.rs index 7617ea5ce3..ae68f15b12 100644 --- a/src/utils/xcode.rs +++ b/src/utils/xcode.rs @@ -59,7 +59,7 @@ where static ref SEP_RE: Regex = Regex::new(r"[\s/]+").unwrap(); } - expand_vars(&s, |key| { + expand_vars(s, |key| { if key.is_empty() { return "".into(); } @@ -174,7 +174,7 @@ impl XcodeProjectInfo { let name = name.to_lowercase(); for cfg in &self.configurations { if cfg.to_lowercase() == name { - return Some(&cfg); + return Some(cfg); } } None @@ -258,10 +258,10 @@ impl InfoPlist { }; // expand xcodevars here - rv.name = expand_xcodevars(&rv.name, &vars); - rv.bundle_id = expand_xcodevars(&rv.bundle_id, &vars); - rv.version = expand_xcodevars(&rv.version, &vars); - rv.build = expand_xcodevars(&rv.build, &vars); + rv.name = expand_xcodevars(&rv.name, vars); + rv.bundle_id = expand_xcodevars(&rv.bundle_id, vars); + rv.version = expand_xcodevars(&rv.version, vars); + rv.build = expand_xcodevars(&rv.build, vars); Ok(rv) }