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.

Sign up for GitHub

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

Set version with git #735

Merged
merged 3 commits into from
Aug 2, 2022
Merged

Set version with git #735

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2782ab0
Get app version using VERSION env or git script
arbulu89 Aug 1, 2022
13319a8
Update suse container and ci to set the version
arbulu89 Aug 1, 2022
6500dcd
Apply better elixir conventions
arbulu89 Aug 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .github/workflows/ci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,10 @@ jobs:

- name: Set version
run: |
VERSION=${{ steps.latest-tag.outputs.tag }}
git config --global --add safe.directory /__w/web/web
VERSION=$(./hack/get_version_from_git.sh)
# "+" character is not allowed in OBS dockerfile version strings
VERSION=${VERSION//[+]/-}
sed -i 's~%%VERSION%%~'"${VERSION}"'~' packaging/suse/Dockerfile

- name: Commit on OBS
Expand Down
3 changes: 3 additions & 0 deletions hack/get_version_from_git.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
#!/bin/sh
set -e
set -o pipefail

TAG=$( git tag | grep -E "[0-9]\.[0-9]\.[0-9]" | sort -rn | head -n1 )

if [ -n "${TAG}" ]; then
Expand Down
17 changes: 16 additions & 1 deletion mix.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,26 @@
defmodule Trento.MixProject do
use Mix.Project

@version "1.1.0"
defp get_version do
arbulu89 marked this conversation as resolved.
Show resolved Hide resolved
case System.get_env("VERSION") do
arbulu89 marked this conversation as resolved.
Show resolved Hide resolved
nil -> get_version_from_git()
version -> version
end
end

defp get_version_from_git do
case File.cwd!() |> Path.join("hack/get_version_from_git.sh") |> System.cmd([]) do
Copy link
Member

@nelsonkopliku nelsonkopliku Aug 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I am wondering whether this is necessary here.
What I mean is that we're setting the VERSION env variable at build time by using get_version_from_git.sh so, do we really need to do that here? 🤔

The alternative I see would be: either at build time the VERSION has been set, or we fallback to the default version.

Am I missing something? would that work? Would that even make sense?

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, I guess you are right. I just implemented this way to have the way to debug it in my development environment.
We can delete this case, no issues with it.

Ok, I will remove it for the sake of simplicity

Copy link
Contributor Author

@arbulu89 arbulu89 Aug 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Edit: Ok, now I know why I have added it...
It was for the community version. The VERSION replacement only affect the suse container, but the community doesn't have such a replacement or Dockerfile edition, and the docker/build-push-action@v3 job doesn't make it that straightforward.

@nelsonkopliku This makes me reconsider the removal of the script usage in the code hehe, as I don't like having string replacements in the community dockerfile. I will check what I can do there, maybe I can use env variables or something like that...

Copy link
Member

Choose a reason for hiding this comment

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

Oh I see, is that the reason why e2e tests are failing?

Anyway, no strong opinion from my side. It is just fine like you did. 💪

Copy link
Member

@fabriziosestito fabriziosestito Aug 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

and the docker/build-push-action@v3 job doesn't make it that straightforward.

ah I see, have to say I am not a big fan of executing shell scripts from elixir but if there is not another way i am ok with it

{version, 0} -> version |> String.trim("\n")
_ -> @version
end
end

def project do
[
app: :trento,
description: "Easing your life in administering SAP applications",
version: "1.1.0",
version: get_version(),
elixir: "~> 1.13",
elixirc_paths: elixirc_paths(Mix.env()),
compilers: [:gettext] ++ Mix.compilers(),
Expand Down
2 changes: 1 addition & 1 deletion packaging/suse/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ ENV MIX_ENV=prod
ENV MIX_HOME=/usr/bin
ENV FLAVOR="Premium"
RUN mix phx.digest
RUN mix release
RUN VERSION=%%VERSION%% mix release

FROM bci/bci-base:15.3 AS trento
# Define labels according to https://en.opensuse.org/Building_derived_containers
Expand Down