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.

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

feat: detect changes in sources #460

Merged
merged 10 commits into from
Dec 11, 2024
Merged

Conversation

AtomicFS
Copy link
Collaborator

@AtomicFS AtomicFS commented Dec 4, 2024

  • few fixes (mostly small stuff)
  • speed up filesystem.DirTree (I just happened to stumble upon it)
  • add functions to detect changes in files
    • add new filesystem functions which can detect changes in files base on time-stamps
    • since we work with large projects, such as Linux Kernel, this detection should be as fast as possible
    • to make it fast, we solely rely on time, no hashing
    • the idea is that firmware-action will create a time-stamp file at the end of each execution (for each target)
    • at the start of each execution firmware-action will load time-stamp file from previous execution and check if any file in sources is newer that the saved time-stamp
    • the detection function is lazy, and will return on first positive match
    • if the no file in sources is newer, target should be considered up-to-date and skipped
  • add the ability to detect changes in sources
  • in addition, the dagger setup was moved so now the loop for up-to-date module is much faster
  • prettify summary table

Summary before:

[INFO   ] Build summary:
uroot-example: Skipped
linux-example-with-uroot: Skipped
coreboot-example-with-linuxboot: Skipped

Summary now:

[INFO   ] Build summary:
+---------------------------------+------------+
| MODULE                          | STATUS     |
+---------------------------------+------------+
| uroot-example                   | Up-to-date |
| linux-example-with-uroot        | Up-to-date |
| coreboot-example-with-linuxboot | Up-to-date |
+---------------------------------+------------+

fixes #441

@AtomicFS AtomicFS requested a review from MDr164 as a code owner December 4, 2024 18:54
@github-actions github-actions bot added feature New feature or request testing Testing related dependencies Pull requests that update a dependency file go Pull requests that update Go code labels Dec 4, 2024
@AtomicFS AtomicFS enabled auto-merge December 4, 2024 19:00
@AtomicFS AtomicFS force-pushed the feat/detect-changes-in-sources branch 3 times, most recently from 9d294db to e8a6ad8 Compare December 5, 2024 14:00
@AtomicFS
Copy link
Collaborator Author

AtomicFS commented Dec 5, 2024

@MDr164 I just made one small change - when the givenTime is zero (for example when there is no timestamp file), it behaves like the target it is up-to-date. This is mostly for convenience in CI.

up-to-date just means that if the output directory exists, it will not re-build.

@AtomicFS
Copy link
Collaborator Author

AtomicFS commented Dec 5, 2024

No to toot my own horn, but I like that this is actually much faster than Taskfile. They go the slow way file-by-file, which this does the directory at the time. And with minimal reads too thanks to the WalkDir (improvement over Walk).

Fully checked out uroot, linux and coreboot are checked in 0.5 seconds.

- should not make difference, but since we have verification running
  over the values, and directories must end with slash, this should
  better match our actual use-case

Signed-off-by: AtomicFS <[email protected]>
- not sure how or why, but we were checking for existing artifacts,
  instead of just checking if the output directory exists
- this fixes the problem

Signed-off-by: AtomicFS <[email protected]>
- it can easily happen that the output directory will be empty, for
  example when build succeeds but the output files are not found
- with this patch, to skip the build the directory can exists, but must
  not be empty

Signed-off-by: AtomicFS <[email protected]>
- replace Walk with WalkDir

Signed-off-by: AtomicFS <[email protected]>
- add new filesystem functions which can detect changes in files base on
  time-stamps
- since we work with large projects, such as Linux Kernel, this
  detection should be as fast as possible
- to make it fast, we solely rely on time, no hashing
- the idea is that firmware-action will create a time-stamp file at the
  end of each execution (for each target)
- at the start of each execution firmware-action will load time-stamp
  file from previous execution and check if any file in sources is newer
  that the saved time-stamp
- the detection function is lazy, and will return on first positive
  match
- if the no file in sources is newer, target should be considered
  up-to-date and skipped

Signed-off-by: AtomicFS <[email protected]>
- this should add the ability to detect changes in sources
- in addition, the dagger setup was moved so now the loop for up-to-date
  module is much faster

Signed-off-by: AtomicFS <[email protected]>
- we wanted to skip .git directory
- submodules contain .git file
- returning 'filepath.SkipDir' on '.git' file skips the directory
  in which the file is

Signed-off-by: AtomicFS <[email protected]>
@AtomicFS AtomicFS force-pushed the feat/detect-changes-in-sources branch from 0be9af6 to 0150e37 Compare December 11, 2024 10:14
@AtomicFS
Copy link
Collaborator Author

@MDr164 do you have time for review?

Copy link
Collaborator

@MDr164 MDr164 left a comment

Choose a reason for hiding this comment

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

Looks mostly fine, just some minor comments

action/filesystem/filesystem.go Outdated Show resolved Hide resolved
action/recipes/recipes.go Outdated Show resolved Hide resolved
@AtomicFS AtomicFS requested a review from MDr164 December 11, 2024 13:25
@AtomicFS AtomicFS added this pull request to the merge queue Dec 11, 2024
Merged via the queue into main with commit ad462d3 Dec 11, 2024
67 checks passed
@AtomicFS AtomicFS deleted the feat/detect-changes-in-sources branch December 11, 2024 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file feature New feature or request go Pull requests that update Go code testing Testing related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rebuild module when sources changed
2 participants