-
Notifications
You must be signed in to change notification settings - Fork 871
feat(windows/craft): deploy PDB debug symbol files when using craft #8423
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
Conversation
56777f9
to
309aa00
Compare
craftmaster-CI.ini
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
having another CraftMaster config in here is a bit too much duplication for my taste :D
I think we should add a new variable to the [Variables]
section instead, reference it inside craftmaster.ini
as ${Variables:MyVar}
, and pass --variables MyVar=true
to the CraftMaster command line parameters (before the -c
flag, everything after that is passed to the Craft installation created by CraftMaster as-is).
This would allow us to easily decide to whether we want to create a build with debug symbols enabled or without.
Another less-flexible way which will only be useful for CI would be to only add config overrides that are relevant for CI here, i.e. replace this entire file with:
[windows-msvc2022_64-cl]
Packager/DownloadDebugSymbolsCache = False
and change the invocation of CraftMaster.py to include both the default craftmaster.ini
and the new craftmaster-CI.ini
:
# [...]
CRAFT_MASTER_CONFIG: ${{ github.workspace }}\craftmaster.ini
CRAFT_MASTER_CONFIG_CI: ${{ github.workspace }}\craftmaster-CI.ini
# [...]
shell: pwsh
run: |
# [...]
function craft() {
python "${{ env.CRAFT_MASTER_LOCATION }}\CraftMaster.py" --config "${{ env.CRAFT_MASTER_CONFIG }}" --config-override "${{ env.CRAFT_MASTER_CONFIG_CI }}" --target ${{ env.CRAFT_TARGET }} -c $args
# [...]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but I think we could even do something better instead
a54f0fe
to
8acc2d0
Compare
|
8acc2d0
to
605cbf4
Compare
we have a limit in file size to 32 bits integer this file is more than 2 GBs Signed-off-by: Matthieu Gallien <[email protected]>
605cbf4
to
28b0c34
Compare
Artifact containing the AppImage: nextcloud-appimage-pr-8423.zip Digest: To test this change/fix you can download the above artifact file, unzip it, and run it. Please make sure to quit your existing Nextcloud app and backup your data. |
|
will enable having proper PDB files deployed when using craft to deploy client dependencies on Windows