Skip to content

Automated build#855

Closed
nefarius2001 wants to merge 10 commits intojamulussoftware:masterfrom
nefarius2001:automated_build
Closed

Automated build#855
nefarius2001 wants to merge 10 commits intojamulussoftware:masterfrom
nefarius2001:automated_build

Conversation

@nefarius2001
Copy link
Copy Markdown
Contributor

@nefarius2001 nefarius2001 commented Jan 18, 2021

Resolves; #854 & #835
Also relates to: #853
intentionally not solving #856 & #857
Credit and thanks to @ann0see for contributing & support

!! DO NOT MERGE YET !!!
this pull request is for preview & discussion
please do not merge until @ann0see and I sorted out Windows & Android builds both working :)

@nefarius2001 nefarius2001 mentioned this pull request Jan 18, 2021
@ann0see
Copy link
Copy Markdown
Member

ann0see commented Jan 18, 2021

I will look into it later ;-).

Windows support (see my PR). Can you move your Android scripts to /android?

Comment thread README.md
![CodeQL](https://github.com/corrados/jamulus/workflows/CodeQL/badge.svg)

Jamulus - Internet Jam Session Software
Jamulus - Internet Jam Session Software
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is that?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

To generate new commits and trigger the ci...
Please ignore/ revert that!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You should probably ensure that you trigger it differently and open a new PR with only working commits.

@nefarius2001
Copy link
Copy Markdown
Contributor Author

nefarius2001 commented Jan 18, 2021

moved build to a subdirectory of "android" for consistency with current structure.
Not perfect consistent, but I would still splitting source code and build scripts for different os ( #856 ).

Copy link
Copy Markdown
Member

@ann0see ann0see left a comment

Choose a reason for hiding this comment

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

Some comments

push:
tags:
- "r*" # run this action if a tag beginning with r is created
workflow_dispatch:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not sure if we should allow workflow_dispatch (for me it didn't always work)

@@ -0,0 +1,39 @@
#!/usr/bin/python3
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Python. Interesting. So you get some information from the current build/tag which triggered this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes.
Easier string manipulations like startsWith.

release_version_name = jamulus_version

if fullref.startswith("refs/tags/"):
print('python is Tag')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this a debugging output?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes, changed to a more meaningful text

PUSHED_NAME=${GITHUB_REF#refs/*/}

# calculate various variables
python3 ${1}/.github/actions_scripts/analyse_git_reference.py ${GITHUB_REF} ${PUSHED_NAME} ${JAMULUS_VERSION}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would it be possible to also get the Jamulus version and Tag via python? this would be much cleaner.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

possible, but more part of #854 optimizations.

jobs:

## Creates the releases on GitHub ##
jobs:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You've changed a lot and I still have to look through it. But it seems to work. Would it be possible that you add the same comments as I did?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done bf47be2

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

at lease I did my best, feel free to adjust them to your needs

run: sh linux/autorelease_linux.sh ${{ github.workspace }}
if: matrix.config.os == 'ubuntu-18.04'
if: matrix.config.target_os == 'linux'
- name: "Build (Windows)"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should be removed and synced to my changes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

how? please make a suggestion

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If you've removed the Install Qt steps (I think this is now the case) and run my script autobuild_windows.ps1 this is solved. I think this PR is no longer up to date.

Comment thread README.md
![CodeQL](https://github.com/corrados/jamulus/workflows/CodeQL/badge.svg)

Jamulus - Internet Jam Session Software
Jamulus - Internet Jam Session Software
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You should probably ensure that you trigger it differently and open a new PR with only working commits.

@@ -0,0 +1,82 @@
FROM ubuntu:18.04
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can't say much from here on (I don't know android building and docker)


# Download / install Qt
####ADD https://code.qt.io/cgit/qbs/qbs.git/plain/scripts/install-qt.sh ./
COPY install-qt.sh /install-qt.sh
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Install-qt.sh that's interesting.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Mmmm. We do need to sort this out so we've a build script (that just gets the pre-reqs and does the qmake-make) per platform so it's re-usable - then the packaging and code analysis can both use it as they see fit.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

that is likely possible.
Though I also see some benefits in using a docker.
Maybe splitting in "installing qt and pre-reqs" and "qmake-make" can be done, to allow caching of the first step?

@@ -0,0 +1,339 @@
#!/usr/bin/env bash
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you also use aqt to install qt? This script looks quite complicated to me.

Copy link
Copy Markdown
Collaborator

@pljones pljones Jan 18, 2021

Choose a reason for hiding this comment

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

I tried copying your aqt method to the CodeQL workflow... without success. Maybe a bad dependency...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Don't know. I was really happy to have something working.
make it work, make it fast, make it clean...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Strange. For me, it worked. Maybe the server is down? At least it says something like this in the CodeQl action at the moment.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

# calculate various variables
python3 ${1}/.github/actions_scripts/analyse_git_reference.py ${GITHUB_REF} ${PUSHED_NAME} ${JAMULUS_VERSION}

perl ${1}/.github/actions_scripts/getChangelog.pl ${1}/ChangeLog ${VERSION} > ${1}/autoLatestChangelog.md
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You will have to rename ${VERSION} to ${JAMULUS_VERSION} if you also renamed the variable above.

Also Codacy complained about missing quotes. We might need to add them around the variables/file paths.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

thanks.
Where can i see these complaints when I develop something?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You will not see them. I setup codacy on my repo only. You can just enable it if you create a codacy account (just google for it).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry guys, I accidentally used the PR branch for development. I will open a new request soon.

@nefarius2001
Copy link
Copy Markdown
Contributor Author

Sorry guys, I accidentally used the PR branch for development. I will open a new request soon.

@nefarius2001 nefarius2001 deleted the automated_build branch January 24, 2021 03:29
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.

4 participants