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

CI improvements #1091

Merged
merged 10 commits into from
Nov 27, 2024
Merged

CI improvements #1091

merged 10 commits into from
Nov 27, 2024

Conversation

dsm
Copy link
Collaborator

@dsm dsm commented Nov 24, 2024

Hi @ra3xdh, this PR;

  • add MSVC test build to deploy.yml (can't build with qucsator_rf because autogenerated files needs post processing to replace somethings with sed tool. Adding pwsh script solve this).
  • move qt5 test build to deploy.yml.
  • some linker options for MSVC.

@zergud
Copy link
Collaborator

zergud commented Nov 24, 2024

@dsm
i have a suggestion:

  1. instead of hardcoded CI_VERSION: 24.4.99 try to take 24.4 part from current qucs-s version
  2. add git commit hash to assets name in CI build, eg Qucs-S-24.4.99-5d47d79-macOS.dmg
    this will help distinguish CI versions

@dsm
Copy link
Collaborator Author

dsm commented Nov 24, 2024

@dsm i have a suggestion:

  1. instead of hardcoded CI_VERSION: 24.4.99 try to take 24.4 part from current qucs-s version
  2. add git commit hash to assets name in CI build, eg Qucs-S-24.4.99-5d47d79-macOS.dmg
    this will help distinguish CI versions
  1. I retrieve the version from the VERSION file without any issues.
  2. The app title includes the commit hash, but the artifact name does not. On macOS, we use Homebrew to install the snapshot builds. Since I only update the version number periodically, we always have the latest builds. However, if the commit hash is part of the file name, it would need to be updated with every new commit to the repository.

@zergud
Copy link
Collaborator

zergud commented Nov 24, 2024

  1. I retrieve the version from the VERSION file without any issues.
  2. The app title includes the commit hash, but the artifact name does not. On macOS, we use Homebrew to install the snapshot builds. Since I only update the version number periodically, we always have the latest builds. However, if the commit hash is part of the file name, it would need to be updated with every new commit to the repository.
  1. Thanks a lot
  2. "Since I only update the version number periodically" - version 24.4.99 never changed before next release
    git hash in night builds used by many projects, for example https://downloads.kicad.org/kicad/windows/explore/nightlies

@ra3xdh
Copy link
Owner

ra3xdh commented Nov 24, 2024

move qt5 test build to deploy.yml

The Qt5 support will be removed in the next year. See #983. I would advise don't touch Qt5 related stuff, because it is a candidate for removing.

add MSVC test build to deploy.yml (can't build with qucsator_rf because autogenerated files needs post processing to replace somethings with sed tool. Adding pwsh script solve this)

The MSVC target must present only for research purposes. The CI should only give a warning but not error if this compiler fails. I am not sure if this is possible. Many of Qucs-S contributors don't own the license for MS development tools. I also have no wish to purchase license. I remember that MSVC had some difference in C++ standards support in the past. The Qt also had some MSVC specific problems. The blocking error from CI MSVC target may potentially create difficulties in accepting contribution. And the Windows release package should be built only with free compilers (MSYS2+MinGW environment)

some linker options for MSVC.

This may be accepted. It doesn't interfere other compilers.

@dsm
Copy link
Collaborator Author

dsm commented Nov 24, 2024

@ra3xdh Visual Studio has community edition so we can use on public project with no problem(we don't need any license). I'll try to solve any problem with MSVC, yes default MSVC compiler isn't match C++ standarts but it has many flags to match C++ standarts. I want MSVC because of Windows on ARM it is easy to cross compile on x64 machine. I move qt5 to deploy.yml because of management sometimes Qt5 build failed but we don't realize it. We can easily removed in deploy.yml when we need.

@dsm
Copy link
Collaborator Author

dsm commented Nov 24, 2024

  1. "Since I only update the version number periodically" - version 24.4.99 never changed before next release
    git hash in night builds used by many projects, for example https://downloads.kicad.org/kicad/windows/explore/nightlies

we have a macos brew in this link [email protected]. I have to change just version number if it's increase but we have a commit hash in file name I have to change this file for every new commits. So this problem we put hash to application tile to identify which version. I can add hash to others but not macos.

@dsm
Copy link
Collaborator Author

dsm commented Nov 24, 2024

The CI should only give a warning but not error if this compiler fails. I am not sure if this is possible.

I added a commit for this.

@ra3xdh
Copy link
Owner

ra3xdh commented Nov 24, 2024

The CI should only give a warning but not error if this compiler fails. I am not sure if this is possible.
I added a commit for this.

Yes, this is acceptable.

@ra3xdh ra3xdh added the CI label Nov 24, 2024
@ra3xdh ra3xdh added this to the 25.1.0 milestone Nov 24, 2024
@dsm
Copy link
Collaborator Author

dsm commented Nov 24, 2024

@ra3xdh Should I add short hash to filenames (except macos because of homebrew)

@ra3xdh
Copy link
Owner

ra3xdh commented Nov 25, 2024

@dsm Yes, adding hash to continuous build packages may be useful to distinguish different builds.

@Kreijstal
Copy link

is there something wrong with msys2 builds?

@dsm
Copy link
Collaborator Author

dsm commented Nov 25, 2024

No, it is just an alternative build. I am experimenting.

@ra3xdh
Copy link
Owner

ra3xdh commented Nov 26, 2024

@Kreijstal The release build must remain on the free development tools (MSYS2+MinGW). We have no plans to switch it to the MSVC. The MSVC build is only for experiments with another compilers. It is marked as continue-on-error and doesn't block merging.

@dsm
Copy link
Collaborator Author

dsm commented Nov 26, 2024

@ra3xdh I need to test this changes one or two more cases.

@dsm
Copy link
Collaborator Author

dsm commented Nov 26, 2024

@ra3xdh Hi, I finish my test.

@ra3xdh ra3xdh merged commit b826d12 into ra3xdh:current Nov 27, 2024
8 checks passed
@ra3xdh
Copy link
Owner

ra3xdh commented Nov 27, 2024

Merged. The MSVC build should be available now.

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

Successfully merging this pull request may close these issues.

4 participants