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

Add no-module-prefix option #10

Merged
merged 1 commit into from Jun 9, 2022

Conversation

Herrie82
Copy link
Contributor

@Herrie82 Herrie82 commented Jun 1, 2022

There are other distributions besides SFOS that are using libconnman-qt via meta-qt5 Yocto layer (https://github.com/meta-qt5/meta-qt5/blob/master/recipes-connectivity/libconnman-qt/libconnman-qt5_git.bb) for example.

Currently meta-qt5 carriers a patch for removing the prefix (https://github.com/meta-qt5/meta-qt5/blob/master/recipes-connectivity/libconnman-qt/libconnman-qt5/0001-Don-t-use-MeeGo-as-prefix-in-order-to-make-this-a-co.patch), a cleaner solution is to upstream it by means of a config option which we're doing now.

Can be enabled by building using "no-module-profix" config option. It should have no influence on the current operation for SFOS.

Signed-off-by: Herman van Hazendonk github.com@herrie.org

@Herrie82 Herrie82 changed the title Add no-prefix option Add no-module-prefix option Jun 1, 2022
Copy link
Member

@rainemak rainemak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spec doesn't have option to pass "no-module-prefix" to the qmake and/or adjusting libdir of the connman-qt5-declarative. That can be done later if needed.

Added minor for the noprefix plugins.qmltypes, other than that this looks good to me.

plugin/files/noprefix/plugins.qmltypes Outdated Show resolved Hide resolved
@Thaodan
Copy link

Thaodan commented Jun 2, 2022

I wonder if it makes sense to remove the prefix altogether.

@rainemak
Copy link
Member

rainemak commented Jun 2, 2022

I wonder if it makes sense to remove the prefix altogether.

As of now, better to have the option for both.

@Herrie82
Copy link
Contributor Author

Herrie82 commented Jun 2, 2022

I'm not a big fan of the duplicated files, but I didn't really find an option to manipulate the qmldir and plugins.qmltypes files to work around this using qmake

@rainemak
Copy link
Member

rainemak commented Jun 2, 2022

I'm not a big fan of the duplicated files, but I didn't really find an option to manipulate the qmldir and plugins.qmltypes files to work around this using qmake

If you'd like to get rid of duplicates, qmldir and plugins.types could have some find-and-replace variables and those variables could be replaced from the project file by with using system command e.g with sed. Downside being that using sed would be make this platform dependant but I doubt that this is much used in Windows.

@Herrie82
Copy link
Contributor Author

Herrie82 commented Jun 2, 2022

that

That sounds like something I'd like as well. Happy for suggestions. sed should work. I can do it in our build system (yocto/bitbake), but this wouldn't work with rpm and I don't know how to do it in qmake to be honest

@pvuorela
Copy link
Contributor

pvuorela commented Jun 2, 2022

On many qml modules we've done module renamings by adding backward compatibility symlinks etc, and if ok then at some point removed the compatibility support. E.g. nemo-qml-plugin-dbus still has both org.nemomobile.dbus and Nemo.DBus.

Here it could be nice if there was a similar bath to gradually migrate to imports without MeeGo.

@rainemak
Copy link
Member

rainemak commented Jun 2, 2022

that

That sounds like something I'd like as well. Happy for suggestions. sed should work. I can do it in our build system (yocto/bitbake), but this wouldn't work with rpm and I don't know how to do it in qmake to be honest

That doesn't need to affect the spec at all. I was thinking something like for plugin.pro:

no-module-prefix: {
    system("sed -i 's/##ReplaceMe##//' qmldir")
    MODULENAME = Connman
} else {
    system("sed -i 's/##ReplaceMe##/MeeGo./' qmldir")
    MODULENAME = MeeGo/Connman
    DEFINES += USE_MODULE_PREFIX
}

And qmldir would be like:

module ##ReplaceMe##Connman
plugin ConnmanQtDeclarative
typeinfo plugins.qmltypes

Feel free suggest something else for the ##ReplaceMe##. Then of course similar steps for the plugins.qmltypes. Take above sample code with a grant of salt as I only ran sed from the command line.

@rainemak
Copy link
Member

rainemak commented Jun 2, 2022

Example of @pvuorela suggestion can be found from nemo-qml-plugin-dbus.

To be noted that we do symlinking and seding of the qlmdir on the spec. As you are not working with rpm / spec, I can propose spec changes on top yours once this is landed.

@Herrie82
Copy link
Contributor Author

Herrie82 commented Jun 8, 2022

@rainemak @pvuorela @Thaodan I pushed a commit to get rid of the duplicate files. Took me a while to get the syntax of sed right. Seems it didn't like the ## and the . after Meego. This works at my end now.

@rainemak
Copy link
Member

rainemak commented Jun 8, 2022

@rainemak @pvuorela @Thaodan I pushed a commit to get rid of the duplicate files. Took me a while to get the syntax of sed right. Seems it didn't like the ## and the . after Meego. This works at my end now.

Could you please squash your changes into one commit. Change itself looks good now.

Upstreams the patch to have a module without Meego prefix as per https://github.com/meta-qt5/meta-qt5/blob/master/recipes-connectivity/libconnman-qt/libconnman-qt5/0001-Don-t-use-MeeGo-as-prefix-in-order-to-make-this-a-co.patch

Signed-off-by: Herman van Hazendonk <github.com@herrie.org>
@Herrie82
Copy link
Contributor Author

Herrie82 commented Jun 8, 2022

@rainemak @pvuorela @Thaodan I pushed a commit to get rid of the duplicate files. Took me a while to get the syntax of sed right. Seems it didn't like the ## and the . after Meego. This works at my end now.

Could you please squash your changes into one commit. Change itself looks good now.

Done :) Thanks for pointers and feedback

@rainemak rainemak merged commit 1b1ef56 into sailfishos:master Jun 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants