Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE REQUEST] Allow to specify dependencies on the manifest. #120

Open
azchohfi opened this issue Mar 31, 2022 · 7 comments
Open

[FEATURE REQUEST] Allow to specify dependencies on the manifest. #120

azchohfi opened this issue Mar 31, 2022 · 7 comments
Assignees
Labels
enhancement New feature or request

Comments

@azchohfi
Copy link
Contributor

💬 Description

A clear and concise description of what you would like to be added to Msix.
Not the same as #108. This request is to be able to add a PackageDependency on the Dependencies group of the Package, literally inside the appxmanifest, not the app installer.
This will enable many scenarios, but specially plugins that have dependencies on Packages. One example is the Windows App SDK (https://github.com/microsoft/WindowsAppSDK/).
This is the spec for this fields:
https://docs.microsoft.com/en-us/uwp/schemas/appxpackage/uapmanifestschema/element-packagedependency.
We need a way to define, in yaml, a list of PackageDependency, which consist of a Name, a Publisher, a MinVersion, and optionally a MaxMajorVersionTested and a Optional parameter.

❓ Platform

Windows

@YehudaKremer
Copy link
Owner

YehudaKremer commented Mar 31, 2022

Hello @azchohfi

Thank you for the suggestion 👍

The Msix format supports a lot of functionality, and maybe even more in the future,
some of those functionalities are common and some are less common.

I think the Msix-package must contain the common functionalities, so users can create Msix easily and fast.

That put us with the less-common functionalities,
maybe we can find a way to enable them with this package,
a way that doesn't require us to add everything in the package source code and maintain it.

I thought about two ways to do that (for now).

one way is to add additionally to the existing msix: create msix: publish commands, two more commands msix: build, msix: pack.

  • The msix: build command will create all the msix files including the AppxManifest.xml file, but will not pack them yet into Msix format,
    at this point, this lets the user the option to manually (or with his ci/cd script) edit the AppxManifest.xml, so he can add manually unsupported abilities.

  • After that, the command msix: pack will just pack those files to a Msix format.

Sound good?

@azchohfi
Copy link
Contributor Author

Thanks for the quick reply. That is one option, for sure. The other solution I can think of is to use a base file to start with, and then the MSIX package would just help "fill the voids" with info from the YAML definition. The initial template can be optional to provide, so if one is found by the MSIX package, then it uses it, and if not, it uses the default one, making it easier for a developer to migrate forward from older versions.
My thinking to this solution is that this is what msbuild does when you build a csproj for UWP, or simply a packaging project(wapproj). There is a user provided manifest that is edited with what is needed during build time. Most of this is static anyway, so asking developers to use an XML edit task to automate this insertions of likely static data seems like a hard task to ask them.
Both solutions should work, and I belive that in the future we should think of a way to let plugins inject themselves into this process so they can inject something they require, like a package dependency, but this also is a separate and more complex issue.

@YehudaKremer YehudaKremer added the enhancement New feature or request label Apr 7, 2022
@YehudaKremer YehudaKremer self-assigned this Apr 7, 2022
@YehudaKremer
Copy link
Owner

Hello @azchohfi

I publish a new version 3.5.0 with the implementation of #120 (comment),
see the new documentation on that: Unsupported Features.

Thank you for your advice on this 👍

@azchohfi
Copy link
Contributor Author

azchohfi commented Apr 7, 2022

Hello @YehudaKremer.

I'm still trying to wrap my head around this solution.

Is the expectation that we check-in the AppxManifest.xml file, from the build folder, into our source control, or should we automate the xml edit so we can inject what we want during build time (in our CI or something like that)?

Looking at the source, I'm assuming that if we check-in the manifest file this will simply not work, since there is more than just creating the msix file in the build folder. We would need to check-in most of the assets and everything else, build the folder, etc, but then we need to run the command with a build_windows = false, since we definitely shouldn't checkin that. Also, if this is the expectation, then all the great features from this package become then completely unusable. If you want to update the version of your package, automatically or manually, you can't do so through any of the msix:* commands, or through the pubspec.yaml. Also, if we run msix:build again, it will replace the manifest and remove any customization. I'm assuming this is not the expected solution, since it is just unmanageable.

The other route is to not check-in anything from the build folder, but to have an intermediate step that is done manually (or preferably in an automated way), in between the execution of msix:build and msix:pack. The problem with this is that we would need to automate a step to inject static content, which is already what the MSIX package is supposed to do for us. This is not a trivial task. On top of that, the dev loop becomes massively complex. There is no single command you can run to test changes in your app in a packaged scenario.

Did I get anything wrong, or is this really what is expected? If it is, then I propose we try to help developers to inject their customizations with a static, simple solution. Maybe the solution I suggested in #120 (comment)? I'm still not sure, but the current solution seems just too complex to maintain.

@YehudaKremer
Copy link
Owner

YehudaKremer commented Apr 7, 2022

Hello @azchohfi

First, I apologize if I was not clear enough about what I did in version 3.5.0.

in your last comment,

Is the expectation that we check-in the AppxManifest.xml file, from the build folder, into our source control, or should we automate the xml edit so we can inject what we want during build time (in our CI or something like that)?

I'm aiming to "automate the XML" way, so the option to save the AppxManifest.xml (for example) in the source code is irrelevant as you mention

Looking at the source, I'm assuming...

You're right.

You are also right about the complexity of my solution,
that is a little too much to develop automation that every time will update the build files between the msix: build and msix: pack commands.

You got everything right 👍

So if we go back to the original problem is that our Configuration (handle the values from the YAML) class is getting bigger and bigger with every addition, which can include more testing and regression bugs...

I think your solution is smart but it also can get complex
because we don't have an IDE (like Visual Studio) that can help the regular developer that not familiar with all the rules of the AppxManifest.xml syntax, so we potentially can get a lot of errors when trying to Pack the AppxManifest.xml to a msix file,
syntax or mismatch errors.
to make things worse, the makeappx.exe program (doing the packing step) does not always return an error with specific information or even a line number (stack).
Maybe that is why Visual Studio works with a visual editor for the common functionalities of the AppxManifest.xml and for many other less common functionalities, the developer will need to edit them manually in the created AppxManifest.xml file.

So in my solution, we build a full AppxManifest.xml with the msix: build that we know is valid, and then let the user work on it, so if we will get an error on the msix: pack step, the user and us will know that is probably because of his edits and maybe he will know what to look for.

Again, you are right that is not a good solution at all.

Maybe the better way is what you said here

let plugins inject themselves into this process...

So we can say that the current package is the "core", and we can develop a separate pub package (plugins) that use the "core" as a dependency.

for example, I a package called "msix_toast_notification" that implements the toast configuration.
in code the package will call:

import 'package:msix/msix.dart';
var msix = Msix();
msix.build();
/// To do: get Toast yaml values
/// and edit the AppxManifest.xml file
msix.pack();

we need to think about how to do this...

Overall what do you think?

@azchohfi
Copy link
Contributor Author

azchohfi commented Apr 7, 2022

A plugin model would work, but I have absolutely no idea how to achieve this in dart/pub.dev, and believe this would add a lot of complexity. I don't have the required expertise to implement this, or even give any meaningful suggestion.

My initial idea would be that the msix plugin can generate the initial, valid, manifest, and then work on top of it to inject the values during build/package. Making an incorrect change in the manifest with static content seems less likely to happen when compared to an automated way, which seems hard to get right and prone to errors. Makeappx.exe will error anyway, regardless of the method used.

Still, I would not revert your changes in 3.5.x. They have value, but I believe this scenario is too complex to implement for static content (which most of these customizations should be).

My suggestion is to change appx_manifest.dart so that the initial manifestContent variable can be provided by the developers (so it is not necessarily hard-coded). We could add some checks to validate this provided appxmanifest so it doesn't conflict with what we are already injecting, or we just ignore it. That would be an implementation detail.
But my suggestion is to use something like https://pub.dev/packages/xml to inject the fields/attributes/etc that are defined in the yaml file into either the provided manifest, or into the same one we already have in that dart file. This would enable full customization with little effort from developers.

If the provided manifest is not valid to begin with, we would have some validations provided "for free" by the https://pub.dev/packages/xml package, in the form of a XmlParserException.

@azchohfi
Copy link
Contributor Author

@YehudaKremer any feedback on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants