Skip to content

helpers: set_base_lmp_version: work on a copy of the git repo#242

Merged
ricardosalveti merged 3 commits intofoundriesio:masterfrom
quaresmajose:set_base_lmp_version
Jan 11, 2023
Merged

helpers: set_base_lmp_version: work on a copy of the git repo#242
ricardosalveti merged 3 commits intofoundriesio:masterfrom
quaresmajose:set_base_lmp_version

Conversation

@quaresmajose
Copy link
Copy Markdown
Member

@quaresmajose quaresmajose commented Jan 6, 2023

Signed-off-by: Jose Quaresma [email protected]

@doanac
Copy link
Copy Markdown
Member

doanac commented Jan 6, 2023

I'm re-running the build https://app.foundries.io/factories/lmp/targets/1979/runs/build-intel-corei7-64/console with your change.

@doanac
Copy link
Copy Markdown
Member

doanac commented Jan 6, 2023

as noted in slack. This fix is fine, but the real issue is that foundriesio/lmp-manifest@d7a019b is overriding what we do here.

@quaresmajose
Copy link
Copy Markdown
Member Author

quaresmajose commented Jan 9, 2023

as noted in slack. This fix is fine, but the real issue is that foundriesio/lmp-manifest@d7a019b is overriding what we do here.

I already knew that setup-environment was called in lmp/bb-build.sh:9 but I didn't know that it was also in lmp/bb-config.sh:51 and this one will massed up the CI config written in lmp/bb-config.sh

grep setup-environment -Rn
lmp/bb-build.sh:9:source setup-environment build
lmp/bb-config.sh:51:source setup-environment build

quaresmajose
quaresmajose commented Jan 9, 2023
View reviewed changes
Comment thread helpers.sh Outdated
@quaresmajose quaresmajose mentioned this pull request Jan 9, 2023
Merged
MrCry0
MrCry0 reviewed Jan 9, 2023
View reviewed changes
Comment thread lmp/bb-config.sh Outdated
ricardosalveti
ricardosalveti reviewed Jan 9, 2023
View reviewed changes
Comment thread helpers.sh Outdated
doanac
doanac approved these changes Jan 9, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@doanac doanac left a comment

Choose a reason for hiding this comment

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

Looks good to me

MrCry0
MrCry0 approved these changes Jan 9, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@MrCry0 MrCry0 left a comment

Choose a reason for hiding this comment

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

LGTM

ricardosalveti
ricardosalveti approved these changes Jan 10, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@ricardosalveti ricardosalveti left a comment

Choose a reason for hiding this comment

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

Nice, thanks! We just need to validate this on a factory and we should be good to land.

@quaresmajose
Copy link
Copy Markdown
Member Author

quaresmajose commented Jan 10, 2023

Nice, thanks! We just need to validate this on a factory and we should be good to land.

I will run a build with this in one of my factories.

@quaresmajose
Copy link
Copy Markdown
Member Author

quaresmajose commented Jan 10, 2023

Nice, thanks! We just need to validate this on a factory and we should be good to land.

I will run a build with this in one of my factories.

Remove the last tag 89 first from my repo and it is still detect as expected from the upstream version

/srv/oe/.repo/manifests.git.tmp /srv/oe
== 2023-01-10 18:40:10 Running: git remote set-url origin https://github.com/foundriesio/lmp-manifest
|--
== 2023-01-10 18:40:10 Running: git fetch origin --tags --quiet
|--
== 2023-01-10 18:40:11 Base LmP cache version detected as: 89
/srv/oe

I will cleanup it a bit removing all these debug commands and let's just stick with == 2023-01-10 18:40:11 Base LmP cache version detected as: 89

Anyawy we can't test the http mirror on the factory because they are used only in public lmp builds, on the factory we use the previous mechanism over nfs

SSTATE_MIRRORS = "file://.* file:///sstate-cache-mirror/PATH"

@ricardosalveti
Copy link
Copy Markdown
Member

ricardosalveti commented Jan 10, 2023

Anyawy we can't test the http mirror on the factory because they are used only in public lmp builds, on the factory we use the previous mechanism over nfs

That's fine, not so concerned with the public lmp jobs.

@quaresmajose
helpers: set_base_lmp_version: work on a copy of the git repo
406c97d 
Signed-off-by: Jose Quaresma <[email protected]>
@quaresmajose
set_base_lmp_version: LMP_VER -> LMP_VERSION_CACHE
ed47772 
This is to be used only for fetching the sstate cache
and can be incremented that's what happens in the development
version of lmp so is better to use a explicit name for that.

Signed-off-by: Jose Quaresma <[email protected]>
@quaresmajose
set_base_lmp_version: fix LMP_VERSION detection
d0529d3 
The version need to be extracted from the upstream lmp-manifest
otherwise it can come from the user repo that can have invalid
tags for this propose or even not having any tag.

Also change it and use abbrev=0 to get only the tag name and in
this way we can use this as well for the sstate cache.

Signed-off-by: Jose Quaresma <[email protected]>
@ricardosalveti ricardosalveti merged commit 4f231a0 into foundriesio:master Jan 11, 2023
@quaresmajose quaresmajose deleted the set_base_lmp_version branch January 13, 2023 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@doanac doanac doanac approved these changes

@ricardosalveti ricardosalveti ricardosalveti approved these changes

+1 more reviewer

@MrCry0 MrCry0 MrCry0 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@quaresmajose @doanac @ricardosalveti @MrCry0