Improvement: multiple output types

[gilescope]

Would love to have json and html output without invoking twice.

[Grubba27]

Hello there! how hard it would be to try implement this ? i'm kind new to rust but i would love to try make this happen, perhpaps making the output commands as a vector and calling for each item on it ?

[marco-c]

@Grubba27 we are using structopt to handle arguments, see the source at

grcov/src/main.rs

Line 68 in 6d86897

#[derive(StructOpt)]

. You can read structopt documentation on how to support options which can have more than one value.

[Grubba27]

Okay, i think I'm getting some where: what I have done afther reading the docs.
adjusted in Opt:

      ],
        possible_values = &[
            "ade",
            "lcov",
            "coveralls",
            "coveralls+",
            "files",
            "covdir",
            "html",
            "cobertura",
        ],
        multiple=true
    )]
    output_type: Vec<OutputType>, // added Vec but i think what i wanted was something like or vec or just  one type

then in the match part i did this:

// iterated over opt.output_type
// Gave me many errors because of iterator and others. perhaps i'm doing this in the wrong way

opt.output_type
        .iter()
        .map(|f| {
            match f {
                OutputType::Ade => output_activedata_etl(iterator, opt.output_path.as_deref(), demangle),
                OutputType::Lcov => output_lcov(iterator, opt.output_path.as_deref(), demangle),
                OutputType::Coveralls => output_coveralls(
                    iterator,
                    opt.token.as_deref(),
                    opt.service_name.as_deref(),
                    &opt.service_number.unwrap_or_default(),
                    opt.service_job_id.as_deref(),
                    &opt.service_pull_request.unwrap_or_default(),
                    &opt.commit_sha.unwrap_or_default(),
                    false,
                    opt.output_path.as_deref(),
                    &opt.vcs_branch,
                    opt.parallel,
                    demangle,
                ),
                OutputType::CoverallsPlus => output_coveralls(
                    iterator,
                    opt.token.as_deref(),
                    opt.service_name.as_deref(),
                    &opt.service_number.unwrap_or_default(),
                    opt.service_job_id.as_deref(),
                    &opt.service_pull_request.unwrap_or_default(),
                    &opt.commit_sha.unwrap_or_default(),
                    true,
                    opt.output_path.as_deref(),
                    &opt.vcs_branch,
                    opt.parallel,
                    demangle,
                ),
                OutputType::Files => output_files(iterator, opt.output_path.as_deref()),
                OutputType::Covdir => output_covdir(iterator, opt.output_path.as_deref()),
                OutputType::Html => output_html(
                    iterator,
                    opt.output_path.as_deref(),
                    num_threads,
                    opt.branch,
                ),
                OutputType::Cobertura => output_cobertura(
                    source_root.as_deref(),
                    iterator,
                    opt.output_path.as_deref(),
                    demangle,
                ),
            };
        });

pic of the errors:

[TomPridham]

i think you prolly want for_each rather than map since you aren't using the output from the iterator. not sure if there is a better way, but you could put let iterator = iterator.clone(); above the match to avoid the move

[Grubba27]

CovResultIter

does not implement Clone in their struct.. :( but that could worked @TomPridham

[TomPridham]

it might be fine to just call rewrite_paths on each iteration. it should only be called a couple of times at max, so it shouldn't be that big of a perf hit. that would prolly be the path of least resistance. otherwise, you might be able to change the return value from rewrite_paths to return just a Vec instead of an Iter and then call clone().iter() on the vec each iteration. that made a decent amount of errors when i tried it just now, tho

[Grubba27]

I was reading about this, perhaps I could implement copy for this return type with struct ? this makes sense ? like this:

pub type CovResultIter = Box<dyn Iterator<Item = (PathBuf, PathBuf, CovResult)>>; // type that rewrite_path returns
#[derive(Clone)]
pub struct CovResultIterStruct {
    pub result: CovResultIter
}

Calling rewite_paths each times could not compile, what i've written is this...

.for_each(|f| { // => cannot move out of `opt` because it is borrowed E0505 move out of `opt` occurs here
use of partially moved value: `opt` E0382 value used here after partial move Note: partial move occurs because `opt.paths` has type `Vec<std::string::String>`, which does not implement the `Copy` trait
 let result_map = Arc::clone(&result_map);
            let path_mapping = Arc::clone(&path_mapping);
            let result_map_mutex = Arc::try_unwrap(result_map).unwrap();
            let result_map = result_map_mutex.into_inner().unwrap();

            let path_mapping_mutex = Arc::try_unwrap(path_mapping).unwrap();
            let path_mapping = path_mapping_mutex.into_inner().unwrap();
            let iterator = rewrite_paths(
                result_map,
                path_mapping,
                source_root.as_deref(),
                prefix_dir.as_deref(),
                opt.ignore_not_existing,
                &opt.ignore_dir,
                &opt.keep_dir,
                filter_option,
                file_filter, // error -> cannot move out of `file_filter`, a captured variable in an `FnMut` closure E0507 move occurs because `file_filter` has type `grcov::FileFilter`, which does not implement the `Copy` trait
            );
            match f {
                OutputType::Ade => output_activedata_etl(iterator, opt.output_path.as_deref(), demangle),
                OutputType::Lcov => output_lcov(iterator, opt.output_path.as_deref(), demangle),
                OutputType::Coveralls => output_coveralls(
                    iterator,
                    opt.token.as_deref(),
                    opt.service_name.as_deref(),
                    &opt.service_number.unwrap_or_default(),
                    opt.service_job_id.as_deref(),
                    &opt.

[TomPridham]

i personally wouldn't do your first suggestion because it creates an unnecessary wrapper to avoid a separate problem. i also don't think it would work since derive(Clone) only works if all the fields impl Clone and Iterator doesn't. the only function that returns CovResultIter is rewrite_paths. you might be able to just change the signature of rewrite_paths to this pub fn rewrite_paths(...) -> Vec<(PathBuf, PathBuf, CovResult)>. then you can move the Box and iter calls inside the for_each prior to passing it to the various coverage generation functions. does that make sense?

if you call file_filter.clone(), does it compile?

[Grubba27]

file_filter.clone() was able if i give the struc the habilty to clone()

#[derive(Default, Clone)]
pub struct FileFilter {
    excl_line: Option<Regex>,

in the match expression i have this issue with the references(&)

 opt.service_name.as_deref(),
                    &opt.service_number.unwrap_or_default(),// this one
                    opt.service_job_id.as_deref(),
                    &opt.service_pull_request.unwrap_or_default(), // this one
                    &opt.commit_sha.unwrap_or_default(),// and this one gives me this error -> Opttion expects a ref for those values.
// what could be my options in this case ?
cannot move out of `opt.service_number`, as `opt` is a captured variable in an `FnMut` closure E0507 move occurs because `opt.service_number` has type `Option<std::string::String>`, which does not implement the `Copy` trait Help: consider borrowing the `Option`'s content

[TomPridham]

i think if you assign those to variables outside of the loop and only use them as references it should work

let service_number = opt.service_number.unwrap_or_default();
...
&service_number,

[TomPridham]

@Grubba27 it would be easier to provide feedback if you opened a draft pr. that way i could see all surrounding code as well

[Grubba27]

Sure!! i will try this wat you said above but the PR is made in this link

[Grubba27]

t

ried this but it would not work :( it gives me this errors

[penso]

Would love to see this happening, I'm currently using grcov and run it twice, once for cobertura (gitlab CI) and one in JSON to get the full test coverage and display it. And running grcov each time is super slow.

[ahollmann]

There is an open pull request for this issue: #893