Skip to content

ENT-12600: compile: Documented/refactored script#1775

Merged
larsewi merged 5 commits into
cfengine:masterfrom
larsewi:compile
Jul 25, 2025
Merged

ENT-12600: compile: Documented/refactored script#1775
larsewi merged 5 commits into
cfengine:masterfrom
larsewi:compile

Conversation

@larsewi
Copy link
Copy Markdown
Contributor

@larsewi larsewi commented Jun 24, 2025 •
edited
Loading

  • compile: fixed shellcheck issues
  • compile: Reduced verbosity of script
  • compile: Fixed inconsistent indents
  • compile: Placed block terminator on a new line for consistency
  • compile: Documented script

Build with no tests
Build Status

Example output:

$ PROJECT=nova ROLE=hub ./buildscripts/build-scripts/compile
 ---snip---
compile: Debug: Running make in core repo...
compile: Debug: Running make install in core repo...
compile: Debug: Running make in enterprise repo...
compile: Debug: Running make install in enterprise repo...
compile: Debug: Running make in nova repo...
compile: Debug: Running make install in nova repo...
compile: Debug: Running make install in masterfiles repo...

^ Snipped noise from sourced scripts

@larsewi
compile: fixed shellcheck issues
e730841 
Signed-off-by: Lars Erik Wik <lars.erik.wik@northern.tech>
olehermanse
olehermanse reviewed Jun 24, 2025
View reviewed changes
Comment thread build-scripts/compile
#!/bin/sh

. `dirname "$0"`/functions
# Usage: PROJECT=[nova|community] ROLE=[hub|agent] ./buildscripts/build-scripts/compile
Copy link
Copy Markdown
Member

@olehermanse olehermanse Jun 24, 2025

Choose a reason for hiding this comment

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

Not for this PR, but we should probably align this with the names we use elsewhere, i.e.
enterprise instead of nova and client instead of agent.

olehermanse
olehermanse reviewed Jun 24, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@olehermanse olehermanse left a comment

Choose a reason for hiding this comment

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

LGTM but nice if @craigcomstock can review as well.

craigcomstock
craigcomstock reviewed Jun 26, 2025
View reviewed changes
Comment thread build-scripts/compile
Comment thread build-scripts/compile
@olehermanse olehermanse requested a review from vpodzime June 30, 2025 17:59
vpodzime
vpodzime approved these changes Jul 1, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@vpodzime vpodzime 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 otherwise.

Comment thread build-scripts/compile Outdated
Comment thread build-scripts/compile

$MAKE -C "$BASEDIR"/core -k
$MAKE -C "$BASEDIR"/core install DESTDIR="$BASEDIR"/cfengine/dist
echo "$(basename "$0"): Debug: Running make in core repo..."
Copy link
Copy Markdown
Contributor

@vpodzime vpodzime Jul 1, 2025

Choose a reason for hiding this comment

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

Looks like functions deserves some new functions like log_error(), log_debug(),...

Copy link
Copy Markdown
Contributor Author

@larsewi larsewi Jul 24, 2025

Choose a reason for hiding this comment

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

#1813

larsewi added 4 commits July 24, 2025 13:53
@larsewi
compile: Reduced verbosity of script
46aeaaf 
Signed-off-by: Lars Erik Wik <lars.erik.wik@northern.tech>
@larsewi
compile: Fixed inconsistent indents
dedd2d5 
Signed-off-by: Lars Erik Wik <lars.erik.wik@northern.tech>
@larsewi
compile: Placed block terminator on a new line for consistency
03a8568 
Signed-off-by: Lars Erik Wik <lars.erik.wik@northern.tech>
@larsewi
compile: Documented script
646a682 
Ticket: ENT-12600
Signed-off-by: Lars Erik Wik <lars.erik.wik@northern.tech>
@larsewi
Copy link
Copy Markdown
Contributor Author

larsewi commented Jul 24, 2025

Build (no tests)
Build Status

@larsewi larsewi requested a review from craigcomstock July 24, 2025 12:00
vpodzime
vpodzime reviewed Jul 24, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@vpodzime vpodzime left a comment

Choose a reason for hiding this comment

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

The last commit could complicate things a lot when there are compilation issues on exotics. Fixing them one by one can take days...

olehermanse
olehermanse previously requested changes Jul 24, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@olehermanse olehermanse left a comment

Choose a reason for hiding this comment

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

PR title is no longer correct - especially last commit makes it a change in behavior rather than just documenting.

@larsewi
Copy link
Copy Markdown
Contributor Author

larsewi commented Jul 24, 2025 •
edited
Loading

The last commit could complicate things a lot when there are compilation issues on exotics. Fixing them one by one can take days...

Good point @vpodzime ! Dropped the commit

@larsewi larsewi requested a review from olehermanse July 24, 2025 15:22
@larsewi larsewi changed the title ENT-12600: compile: Documented script ENT-12600: compile: Documented/refactored script Jul 24, 2025
@larsewi larsewi dismissed olehermanse’s stale review July 25, 2025 08:09

Changed PR title and dropped last commit

@larsewi larsewi merged commit 657b4c0 into cfengine:master Jul 25, 2025
1 of 2 checks passed
@larsewi larsewi deleted the compile branch October 2, 2025 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vpodzime vpodzime vpodzime approved these changes

@craigcomstock craigcomstock Awaiting requested review from craigcomstock

@olehermanse olehermanse Awaiting requested review from olehermanse

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@larsewi @vpodzime @craigcomstock @olehermanse