Fix licenses workspaces

[serpilliere]

The license is always generated using a None package with results in failing the initialization in workspaces environements.
This is due to:

 let package = package(&manifest, None)?;

This PR adds the package field to the license creation code path in order to forward the desired package option.

[volks73]

Thank you for the PR and the fix. This does appear to be an oversight with the license generation.

Would it be possible to get a unit and/or integration test that would ensure the license builder works with the package argument?

[serpilliere]

Yes of course!
Can you tell me if I am on the correct road?
I looked at the test in tests/initialize.rs, and more precisely in:

#[test]
#[serial]
fn workspace_package_works() {

So I commented all the tests, except this one.
It seems to be a good start point as it seems the code create a workspace with two sub packages.
But if I run it, (linux, main branch) I get:

test workspace_package_works ... FAILED

failures:

---- workspace_package_works stdout ----
thread 'workspace_package_works' panicked at 'assertion failed: result.is_ok()', tests/initialize.rs:833:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

After some debugging, it seems that the generated packages don't have an 'authors' field which is tested.
Moreover, the cargo command author creation doesn't support authors on its comandline project creation.

Did I do something wrong or is this current test broken?

[volks73]

@serpilliere You are on the right track and I would use the tests/initialize.rs as a template and good starting point.

But if I run it, (linux, main branch) I get:

You need WiX Toolset installed, which is a Windows only application, in order to pass some tests. You might run into some problems if you are trying to run the tests on a Linux machine.

After some debugging, it seems that the generated packages don't have an 'authors' field which is tested.
Moreover, the cargo command author creation doesn't support authors on its comandline project creation.

Did I do something wrong or is this current test broken?

You did nothing wrong. The test is broken. More specifically, the fixture is broken, and I have been slacking and not provided a fix yet. Please see #146 for some more information. You are probably using a version of Rust greater than v1.50. If you install an older version, the tests should run without error.

[serpilliere]

Oh! I admin I didn't look at the issues linked to the tests. Sorry about that.
For the windows compilation, I am aware of it: I just wanted to generate the wix, not to compile it.

Here is my understanding for the license regression test:
It seems that the project I am using makes wix fail because it has a "license" field in its cargo file. In the following wix code:

        let (eula_wxs_path, license_wxs_path) = match Eula::new(self.eula.as_ref(), &package)? {

The eula result is an Eula::Generate(template) and this code path makes the license generation with the none package resulting in the error. In orther code path, the license bug is not triggered.

So in my understanding of the code, to make a regression test that triggers the bug, I need at least two conditions:

• a workspace with multiple sub packages
• a license field.

A third condition is also necessary on 'recent' cargo:

• an author field.

It also seems that cargo cannot generate a license field by default. So it seems we face the very same problem to generate a regression test: We need to modify the cargo toml file.

In your existing tests code base, there are such a use case:

        toml.get_mut("package")
            .map(|p| {
                match p {
                    Value::Table(ref mut t) => {
                        t.insert(String::from("license"), Value::from("MIT"))
                    }

So here is my proposition: The new regression test will use a code similar to the èworkspace_package_works` test, but will also customize the generated toml files

Is it ok for you?

[volks73]

@serpilliere Great work! This is all excellent. Your understanding and summary are correct and very useful.

So here is my proposition: The new regression test will use a code similar to the èworkspace_package_works` test, but will also customize the generated toml files

Is it ok for you?

This is okay with me and please proceed.

A separate PR to fix the author field for newer versions of Cargo is going to be needed and we can use your insights as a template for that PR. Thank you.

[serpilliere]

Hi @volks73
I proposed the #166 to first add the authors.
For the license, as it seems a bit complex to do a generic code between authors and license, I propose to add in this pr another function which add a license based on the toml modification in addition of the the tests.

[volks73]

@serpilliere I have merged #166. Thanks for the contribution. With that merge, you should be able to add a function to modify the TOML file for the license as needed.

[serpilliere]

I added the regression test.

[volks73]

Looks good.

Thank you for adding the test.

[serpilliere]

Thank you for the merge!