Skip to content

Don't check for lib and bin paths in sanity check step for each extension - #5221

Merged
boegel merged 2 commits into
easybuilders:developfrom
Flamefire:remove-duplicate-dir-checks
Jul 30, 2026
Merged

Don't check for lib and bin paths in sanity check step for each extension#5221
boegel merged 2 commits into
easybuilders:developfrom
Flamefire:remove-duplicate-dir-checks

Conversation

@Flamefire

@Flamefire Flamefire commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

We currently set defaults for paths checked in sanity check step if:

  • sanity_check_paths is not given or enhance_sanity_check
  • AND custom_paths from the easyblock is not given/empty

This is the case in general, so for a bundle with 100 extensions we will do ~ 200 redundant checks where each will walk the folders to determine if they are empty

This change might miss checking bin & lib now if the easyconfig:

  • uses exts_list
  • enhance_sanity_check is not set
  • sanity_check_paths is used at the top level and does not include bin & lib or subfolders/files, like bin/toy, lib/python
  • (parent) easyblock passes custom_paths without such folders/files
  • all easyblocks used for extensions do not pass custom_paths or do not check for such folders/files

Reason: If e.g. the parent easyconfig/block does pass custom_paths then we will not check the defaults in the parent. And with the change we don't check the default in the extensions anymore and hence may not check it at all.

@Flamefire
Flamefire force-pushed the remove-duplicate-dir-checks branch from 28dfce9 to adaa37a Compare July 27, 2026 11:24
@boegel boegel changed the title Don't check for lib and bin paths for each extension Don't check for lib and bin paths in sanity check step for each extension Jul 29, 2026
@boegel boegel added the bug fix label Jul 29, 2026
@boegel boegel added this to the next release (5.3.2 or 5.4.0) milestone Jul 29, 2026
@boegel

boegel commented Jul 29, 2026

Copy link
Copy Markdown
Member

This change might miss checking bin & lib now if the easyconfig:

* uses `exts_list`

* `enhance_sanity_check` is not set

* `sanity_check_paths` is used at the top level and does not include bin & lib or subfolders/files, like `bin/toy`, `lib/python`

* (parent) easyblock passes `custom_paths` without such folders/files

* all easyblocks used for extensions do not pass `custom_paths` or do not check for such folders/files

Reason: If e.g. the parent easyconfig/block does pass custom_paths then we will not check the defaults in the parent. And with the change we don't check the default in the extensions anymore and hence may not check it at all.

I don't think that's a problem...
If sanity_check_paths is specified without enabling enhance_sanity_check, you should expect that you're in full control, i.e. only the paths listed will be checked.

I don't think people would assume that a check for a non-empty bin and/or lib directory would be done for specific extensions, generally speaking, so I don't expect anyone to actually (silently) rely on this bug .

@Flamefire

Copy link
Copy Markdown
Contributor Author

I've been discussing this with @Crivella on Slack for a while and we tried to find affected EasyConfigs but determining the exact conditions was a bit difficult which is why I opened this as a separate PR so someone else can verify with fresh eyes that a) that list of conditions is correct and complete and b) really doesn't apply to (at least) our easyconfigs.

If sanity_check_paths is specified without enabling enhance_sanity_check, you should expect that you're in full control, i.e. only the paths listed will be checked.

I don't think people would assume that a check for a non-empty bin and/or lib directory would be done for specific extensions, generally speaking, so I don't expect anyone to actually (silently) rely on this bug .

My first suspicion would be something like PythonBundles: Assuming the packages check for bin & lib the bundle sanity_check_paths might only check for a specific path like some "data/foo"
So I guess once we are sure under which conditions this changes anything at all we can check if this applies to any EC

@boegel

boegel commented Jul 29, 2026

Copy link
Copy Markdown
Member

I've been discussing this with @Crivella on Slack for a while and we tried to find affected EasyConfigs but determining the exact conditions was a bit difficult which is why I opened this as a separate PR so someone else can verify with fresh eyes that a) that list of conditions is correct and complete and b) really doesn't apply to (at least) our easyconfigs.

If sanity_check_paths is specified without enabling enhance_sanity_check, you should expect that you're in full control, i.e. only the paths listed will be checked.
I don't think people would assume that a check for a non-empty bin and/or lib directory would be done for specific extensions, generally speaking, so I don't expect anyone to actually (silently) rely on this bug .

My first suspicion would be something like PythonBundles: Assuming the packages check for bin & lib the bundle sanity_check_paths might only check for a specific path like some "data/foo" So I guess once we are sure under which conditions this changes anything at all we can check if this applies to any EC

I would be quite surprised if any easyconfigs would really make that kind of assumption.

And even if they would, nothing would actually break in practice: some sanity check path checks would just not happen anymore, but it wouldn't result in the installation failing. Theoretically, missing paths could be overlooked, but even that is quite unlikely to happen, I would say...

@boegel boegel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@boegel
boegel merged commit 6363ba6 into easybuilders:develop Jul 30, 2026
41 checks passed
@Flamefire
Flamefire deleted the remove-duplicate-dir-checks branch July 30, 2026 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants