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

glscopeclient: Check WIXPATH before creating MSI installer #388

Merged
merged 3 commits into from
Sep 1, 2021
Merged

glscopeclient: Check WIXPATH before creating MSI installer #388

merged 3 commits into from
Sep 1, 2021

Conversation

Biswa96
Copy link
Contributor

@Biswa96 Biswa96 commented Aug 28, 2021

No description provided.

@azonenberg
Copy link
Collaborator

The CI build failed. Do you need to install additional software in the CI environment perhaps? (see the .github directory for the scripts)

@Biswa96
Copy link
Contributor Author

Biswa96 commented Aug 29, 2021

Done. @umarcor

-DCMAKE_INSTALL_PREFIX="${MINGW_PREFIX}" \
-DBUILD_TESTING=OFF \
../
cmake --build .

${MINGW_PREFIX}/bin/cmake.exe --build ./
Copy link
Contributor

Choose a reason for hiding this comment

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

cmake --build ./ should work, isn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, previous command works. The new one is just to play safely.

}

package() {
cd "${srcdir}"/../../build
make DESTDIR="${pkgdir}" install
DESTDIR="${pkgdir}" ${MINGW_PREFIX}/bin/cmake.exe --build ./ --target install
Copy link
Contributor

Choose a reason for hiding this comment

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

DESTDIR="${pkgdir}" cmake --build ./ --target install?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, previous command works. The new one is just to play safely.

COMMENT "Creating Windows x64 MSI..."
DEPENDS dist_windows_x64
WORKING_DIRECTORY ${CMAKE_BINARY_DIR}
COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_SOURCE_DIR}/src/glscopeclient/wix/LICENSE.rtf ${CMAKE_BINARY_DIR}
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems that indentation is not correct in this line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done 👍

Copy link
Contributor

@umarcor umarcor left a comment

Choose a reason for hiding this comment

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

Apart from the style issues (see comments), LGTM!

Copy link
Contributor

@umarcor umarcor left a comment

Choose a reason for hiding this comment

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

Nice work!

@Biswa96
Copy link
Contributor Author

Biswa96 commented Aug 30, 2021

One litte question. Why does the CI compile the program twice?

@azonenberg
Copy link
Collaborator

One litte question. Why does the CI compile the program twice?

It should build once for Linux and once for Windows. If there's multiple builds, then we've got a bug in the scripts.

@umarcor
Copy link
Contributor

umarcor commented Aug 30, 2021

@azonenberg, on Windows it is built twice:

I guess @Biswa96 is asking why don't we extract the .zst for generating the zipfile, instead of creating it from scratch. I guess the answer is that we want to keep the portable zipfile build procedure standalone, so contributos can ignore the PKGBUILD recipe if they want.

@Biswa96
Copy link
Contributor Author

Biswa96 commented Aug 30, 2021

As you are distributing the ZIP file to users, why not skip the msys2 package build or PKGBUILD entirely?

@umarcor
Copy link
Contributor

umarcor commented Aug 30, 2021

The MSYS2/PKGBUILD build is executed for Continuous Integration/Testing purposes. That provides "nightly" artifacts for contributors to test enhancements/PRs, before the package is bumped in MSYS2 repositories.

@Biswa96
Copy link
Contributor Author

Biswa96 commented Aug 30, 2021

OK, all doubt cleared 😌

@azonenberg azonenberg merged commit d95666c into ngscopeclient:master Sep 1, 2021
@Biswa96 Biswa96 deleted the fix-win-build branch September 1, 2021 03:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants