Serial-Studio was upstreamed to MSYS2 repositories about serial-studio HOT 27 CLOSED

@umarcor No problem! Thanks for contributing to this project & for the patience! I'll notify you when I add the option of disabling the automatic updater completely, but that will be in a couple of hours.

from serial-studio.

but that will be in a couple of hours

Or days, weeks. No rush at all, mate! It'll be done when it's done, neither faster nor later 😉

from serial-studio.

Hi @umarcor, first of all, a big thank you for creating the PKGBUILD recipe!

A make install target can be easily generated by modifying the Serial-Studio.pro file (for example, here are the make install commands for GNU/Linux).

I think that we can easily do something similar for the win32* target. I just need to know what values you need for target.path and license.path.

Greetings!

from serial-studio.

Update, I added the following config for win32 in the qmake project:

win32* {
    TARGET = SerialStudio
    RC_FILE = deploy/windows/resources/info.rc
    
    # MSYS2 integration
    target.path = /bin
    license.path = /share/licenses/
    license.files += LICENSE.md
    INSTALLS += target license
}

Please let me know if this configuration allows you to run make install and obtain the appropriate results.

from serial-studio.

@alex-spataru you were so fast! Thanks for your quick and proactive reaction!

I created #9 for iterating faster on this. You should be able to push to that branch (or create another one based on that), if you want.

It seems that the new make install target does work, however, it is ignoring the PREFIX and DESTDIR variables, which are very used by packagers. See https://github.com/Serial-Studio/Serial-Studio/pull/9/files#diff-57c1a8b93f074586ac89e2e3f65f87bc428edc786ae3ca176cba639141dcf95fR25 and https://github.com/Serial-Studio/Serial-Studio/pull/9/files#diff-57c1a8b93f074586ac89e2e3f65f87bc428edc786ae3ca176cba639141dcf95fR35. As a result, install is placing the binaries in the system, and not in the desired location, and the resulting package is empty. See https://github.com/umarcor/Serial-Studio/runs/1732014741?check_suite_focus=true#step:5:1164 and https://github.com/umarcor/Serial-Studio/runs/1732043616?check_suite_focus=true#step:5:18. Compare with the "manual" procedure: https://github.com/umarcor/Serial-Studio/runs/1731960761?check_suite_focus=true#step:5:24.

Therefore, I think that just honouring PREFIX and DESTDIR would probably fix the issue. However, I am not sure about $(DESTDIR)/$(PREFIX)/bin and $(DESTDIR)/$(PREFIX)/share/licenses being the desirable paths for all win32* targets. On the one hand, in the PKGBUILD recipes, the license is copied to $(DESTDIR)/$(PREFIX)/share/licenses/serial-studio/. On the other hand, non MSYS2 builds on Windows might require other locations.

from serial-studio.

In order to have some reference, I found that the fritzing package uses Qt5 (qmake) too, and there is a patch for dealing with these installation paths. See:

• https://github.com/msys2/MINGW-packages/blob/master/mingw-w64-fritzing/PKGBUILD
• https://github.com/msys2/MINGW-packages/blob/master/mingw-w64-fritzing/0003-qmake-fixes-for-mingw.patch

It seems that if(unix:!macx)|mingw, unix|mingw, win32:!mingw, etc. can be used for telling apart MSYS2 (mingw) targets from win32 targets.

from serial-studio.

Hi @umarcor,

I'll check what can be done so that qmake takes into account the PREFIX and DESTDIR variables. It should be possible because on Linux these variables are honored for app image generation.

Regarding:

However, I am not sure about $(DESTDIR)/$(PREFIX)/bin and $(DESTDIR)/$(PREFIX)/share/licenses being the desirable paths for all win32* targets.

And:

It seems that if(unix:!macx)|mingw, unix|mingw, win32:!mingw, etc. can be used for telling apart MSYS2 (mingw) targets from win32 targets.

I think that adding specific qmake instructions for MinGW is the correct approach, since I am using MSVC for the binaries that I distribute through GitHub releases. I will do some research, modify Serial-Studio.pro and notify you as soon as I come up with something.

Greetings!

from serial-studio.

@umarcor I added the following code in Serial-Studio.pro:

#-------------------------------------------------------------------------------
# MSYS2 integration
#-------------------------------------------------------------------------------

win32-g++ {
    target.path = $$(pkgdir)$$(MINGW_PREFIX)/bin
    license.path = $$(pkgdir)$$(MINGW_PREFIX)/share/licenses/$$(_realname)
    license.files += LICENSE.md
    INSTALLS += target license
}

This only affects Windows builds using g++ (basically just MinGW builds AFAIK). You can modify freely that configuration, since I use MSVC for deploying the app (and I don't use make install for creating the NSIS installer).

In this configuration, $$(pkgdir), $$(MINGW_PREFIX) and $$(_realname) are pulled directly from the env. variables, please let me know if this helps you.

from serial-studio.

Hi @alex-spataru! Thanks again for your quick reaction.

I did try the modifications you did, but it does not work as expected. There are two functions in the build:

• build, where qmake is executed and pkgdir is not defined.
• package, where make is executed and pkgdir is defined.

Therefore, your proposed code is interpreted as follows:

win32-g++ {
    target.path = $$(MINGW_PREFIX)/bin
    license.path = $$(MINGW_PREFIX)/share/licenses/$$(_realname)
    license.files += LICENSE.md
    INSTALLS += target license
}

And the result is: https://github.com/Serial-Studio/Serial-Studio/runs/1796167547?check_suite_focus=true#step:5:1161

==> Starting package()...
make -f Makefile.Release install
make[1]: Entering directory '/d/a/Serial-Studio/Serial-Studio/msys2/src/build'
cp -f release/SerialStudio.exe D:/a/_temp/msys/msys64/mingw64/bin/SerialStudio.exe
D:/a/_temp/msys/msys64/mingw64/bin/qmake.exe -install qinstall D:/a/Serial-Studio/Serial-Studio/LICENSE.md D:/a/_temp/msys/msys64/mingw64/share/licenses/LICENSE.md
make[1]: Leaving directory '/d/a/Serial-Studio/Serial-Studio/msys2/src/build'

By looking at the generated makefile:

install_target: first FORCE
	@test -d C:$(INSTALL_ROOT:@msyshack@%=%)/msys64/mingw64/bin || mkdir -p C:$(INSTALL_ROOT:@msyshack@%=%)/msys64/mingw64/bin
	-$(INSTALL_FILE) $(DESTDIR_TARGET) C:$(INSTALL_ROOT:@msyshack@%=%)/msys64/mingw64/bin/$(TARGET)

So, make INSTALL_ROOT="$pkgdir" does neither work.

I asked for help in the MSYS2 community. Hope someone can provide some guidelines.

from serial-studio.

@alex-spataru, I think I found a solution by following the advice in the MSYS2 chat. Please, see #9. Precisely 8484157. Let me know if you can cherry-pick it.

from serial-studio.

@umarcor Looks good to me! I just executed git cherry-pick with your repo. Honestly, I just learned about the existence of that command right now. I still do most of my work with the basic git pull, push and commit commands and do merges with the GitHub UI 🤣.

Anyways, the commands that I executed where:

git remote add other https://github.com/umarcor/Serial-Studio
git fetch other
git checkout master
git cherry-pick 8484157

Please let me know if I executed the right commands.

from serial-studio.

@alex-spataru that's correct! I rebased and pushed again for ensuring that everything works as expected. I got the arbitrary Bad address issue, but other than that it looks good: https://github.com/umarcor/Serial-Studio/actions/runs/535908540 Thanks!

from serial-studio.

@umarcor Perfect! The workflow looks a lot better with your changes. Should I mark your PR as 'ready for review'?

from serial-studio.

@alex-spataru I'm not sure about that. There is that arbitrary Bad address error which might be annoying for you. You will effectively need to restart each CI run several times until it works. Sometimes it's once, sometimes two or three. Therefore, I'd prefer if we focused on SerialSetudio --version or SerialSetudio --help, and on optionally disabling the automatic version check.

At the same time, I will split the workflow enhancements for Linux/macOS, so that you can pick them if you wish.

from serial-studio.

At the same time, I will split the workflow enhancements for Linux/macOS, so that you can pick them if you wish.

Thanks a lot! I just cherry-picked those changes.

Therefore, I'd prefer if we focused on SerialSetudio --version or SerialSetudio --help

Ok, I can add the --version flag without any major problems. I have done this for other projects. However, what should the program do when receiving the --help argument? I think that the easy solution would be to show a menu displaying available commands (like most CLI programs do), or should we add something specific for MSYS2?

The text displayed with --help could be similar to:

Usage: SerialStudio [ options ... ]                 

Options include:
    -b, --bug                            Report a bug
    -h, --help                           Show this message
    -r, --reset                          Reset/clear the settings
    -v, --version                        Display the application version 
    -d, --disable-automatic-updates      Disable automatic update checking

Note that none of these CLI commands have been implemented yet.

from serial-studio.

My main concern is having some "smoke test" for ensuring that the binary was built successfully. Locally, I can execute Serial Studio, because I do have a display. However, I cannot do it CI. Therefore, --version would suffice. No need for --help. There is no specific need for MSYS2 in this regard. This is just for CI.

The specific request for packagers is allowing to disable the automatic version check. That's not exclusive for MSYS2, but for any system package.

from serial-studio.

-d, --disable-automatic-updates Disable automatic update checking

I had overlooked this. I think the option should not be a runtime option, but defined at build time. The point is that the automatic version checker points to the tarballs/zipfiles/installers in the releases of this repo. However, when packaged, users are expected to update the tool using the same package manager they used for installing it. Hence, as soon as you publish a new release and system packages are "outdated", users will intuitively download a different build and have a duplicated setup. Instead, I think that the automatic check/update should be enabled in the binaries you provide in this repo only.

from serial-studio.

@umarcor I just added the option to run Serial Studio with the -v and --version CLI arguments. I'll work on adding a compile definition to disable the auto updater later.

from serial-studio.

Hi @alex-spataru! I just tested it both in CI and locally. The --version command does work partially. The command is properly supported, so no error is produced. However, no output is shown:

# ./pkg/mingw-w64-x86_64-serial-studio/mingw64/bin/SerialStudio.exe --version

# ./pkg/mingw-w64-x86_64-serial-studio/mingw64/bin/SerialStudio.exe -v

https://github.com/umarcor/Serial-Studio/runs/1836326477?check_suite_focus=true#step:5:1167

from serial-studio.

Well, I began investigating what could cause the issue, and I found out that Windows apps have two modes:

• GUI
• Non-GUI (console)

GUI apps cannot display console output (console output, even std::cout or printf is only visible through a debugger). To fix this, you need to define the following in the project file:

CONFIG += console

However, this will cause a console window to be opened alongside the GUI application (and I have a feeling that most users would not approve of that). Here is a screenshot of Serial Studio executed directly from the file explorer (not through cmd.exe):

I found a solution that may work for this case (StackOverflow link). We basically add the following code in main.cpp:

#ifdef Q_OS_WIN
#include <windows.h>
#endif

int main(int argc, char **argv) {
    // Hide console window on Microsoft Windows when there are no CLI arguments
#ifdef Q_OS_WIN
    if( argc < 2 )
        ::ShowWindow( ::GetConsoleWindow(), SW_HIDE );
#endif

    // Proceed to init...
}

However, the console window is still shown for an instant while launching the application (and I really don't like what I am doing here, its an ugly hack just to get basic functionality).

I see two possible solutions to this problem:

• Only define CONFIG += console for MinGW builds (this means MinGW users will always see a console window while using Serial Studio).
• Find a way for the CLI build script to get console output of GUI apps.

Some users recommended making a small non-GUI app that launches the GUI app to get around this issue. But I really think that would be an overkill just to be able to use SerialStudio --version on Windows.

EDIT: Forget what I said earlier, I found a working solution (https://stackoverflow.com/a/41701133). I'll push my changes in a moment.

from serial-studio.

@alex-spataru see https://github.com/umarcor/Serial-Studio/runs/1836612927?check_suite_focus=true#step:5:1167

No appenders registered with logger
[Info   ] <qMain> 
[Debug  ] <> Serial Studio version 1.0.14
[Debug  ] <> Written by Alex Spataru <@alex-spataru>

All I did was add a LOG_INFO(); in a "random" location of main.cpp: umarcor@a1d2116

To me that output is enough. I think the problem with using qDebug alone is that you are not explicitly flushing stdout/stderr before EXIT_SUCCESS.

What you said about console is different. We don't want an explicit console. When users do double-click, we don't want any console to be created. However, here we are calling the binary from a console already. The app only needs to print to stdout/stderr, the same way it prints any other regular message.

from serial-studio.

@alex-spataru I saw your edit now. I rebased and pushed. It works! Thanks a lot, again!

# ./pkg/mingw-w64-x86_64-serial-studio/mingw64/bin/SerialStudio.exe -v

Serial Studio version 1.0.14
Written by Alex Spataru <https://github.com/alex-spataru>

from serial-studio.

I just added the option to disable the updater via preprocessor definitions. The auto-updater is automatically disabled in MinGW builds.

from serial-studio.

Thanks! I will wait until you tag 1.0.14 so I can check the behaviour before and after the fix. Currently, it is uneffective because the version is 1.0.13, so the auto-updater is not triggered.

from serial-studio.

Hi @umarcor, I just released another version. SS versions 1.0.14 & 1.0.15 compiled with MinGW should not warn about today's update.

from serial-studio.

Thanks Alex! I tested it locally and it works nice!

I updated the MSYS2 package and opened a PR: msys2/MINGW-packages#7964.

Do you want to keep this issue open for visibility and future coordination? Or shall we close it?

from serial-studio.

I updated the MSYS2 package and opened a PR: msys2/MINGW-packages#7964.

Thanks a lot! I really appreciate the effort you put into this project.

Do you want to keep this issue open for visibility and future coordination? Or shall we close it?

I think that the best thing to do is to convert this issue into a discussion for visibility and future coordination. I'll do that in a few moments.

from serial-studio.