Skip to content

Local semver#681

Open
oocube wants to merge 17 commits into
masterfrom
localSemver
Open

Local semver#681
oocube wants to merge 17 commits into
masterfrom
localSemver

Conversation

@oocube

@oocube oocube commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Fixes #676 by introducing semver to local builds and adding the BUILDER environment variable.

@mcarans mcarans left a comment

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.

Thanks for looking into this. It's a step in the right direction. See my comments for some requested changes

@@ -0,0 +1,109 @@
#!/bin/bash

@mcarans mcarans Jun 18, 2026

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.

It's great you made a helper script for this. It could probably be put in ShellScripts/common as it looks like it could work on Windows MSYS2 as well.

This should be made a bash function to avoid spawning a subshell. An example is this existing function: https://github.com/OoliteProject/oolite/blob/master/ShellScripts/Linux/dep_location_fn.sh#L3 which is then called like this: https://github.com/OoliteProject/oolite/blob/master/ShellScripts/Linux/build_gnustep.sh#L11-L12.

I add _fn to the script name ie. download_github_fn.sh to distinguish that it is a function to be called by other scripts.

If you want to also be able to call it like a script for your own use in bash, then you can make a separate download_github.sh that sources the function from download_github_fn.sh (but the install_packages_root.sh script should use the function directly to avoid making a subshell).

@@ -13,7 +13,7 @@ jobs:
runs-on: ubuntu-latest

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.

With gitversion called in the build itself in get_version.sh, why is the dependency on common still needed? We need to get rid of the dependency on common so that each Windows and Linux build is self contained otherwise it's hard to replicate build failures that occur in CI locally. If it is informational, then it can run without being a dependency (remove the needs: [common] lines) in which case it should be renamed to something like ci_information.

Comment thread src/SDL/MyOpenGLView.m
{
#ifdef BUILD_DATE
NSString *caption = [NSString stringWithFormat:@"Oolite v%@ - %s", @OO_VERSION_FULL, BUILD_DATE];
NSString *caption = [NSString stringWithFormat:@"Oolite v%@ by %@ - %s", @OO_VERSION_FULL, @OO_BUILDER, BUILD_DATE];

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.

If OoliteProject/oolite is forked and a package is built by CI for the fork, what will OO_BUILDER be? It shouldn't be OoliteProject or whatever name is associated with the main repo.

pushd "$SCRIPT_DIR"

# install gitversion
GITVERSION_TGZ=$(./download_github.sh --owner GitTools --repository GitVersion --filter linux-x --outputdir ${SCRIPT_DIR})

@mcarans mcarans Jun 18, 2026

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.

$() creates a subshell - source download_github_fn.sh and call a function in there called download_github instead

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.

linux packageinfo version is inconsistent with git tags

2 participants