feat: Clean cache command

[d3xter666]

The command completely cleans the cache by removing the cache files as well as cleaning up the SQLite records.
It does not wipe out the SQLite DB file(s)

JIRA: CPOUI5FOUNDATION-891

[RandomByte]

It seems odd that this file is placed next to the other files responsible for managing the framework packages but doesn't use any of them. I would expect better integration here, e.g. no hardcoded assumptions like the framework/ directory name as well as checks for existing lockfiles to prevent deleting files while a download is running

[d3xter666]

I understand your point, but what botters me is that this might need a bit more of a refactoring.

Here's my rationale.

The AbstractInstaller is an abstract class that implements the lock logic. As the name suggests- it's an installer that is extended by the npm and maven installers.
Cache clanup, except from having in common the locks and paths, is a completelly different topic- it needs to clean the framework files. I have also seen that the framework folder is not configured in the AbstractInstaller, but is hardcoded in every installer.

The only clean option I forsee is to consolidate and reuse the locking logic and abstract the "framework" dir within the AbstractInstaller. Then somehow reuse this information in the cache cleaner.

[d3xter666]

I have refactored the code, so that framework folder is reused accross classes and the locking is respecte.
Hopefully, this change addresses you comment: 8a53eb8

Let me know if you have other concerns on that matter

[RandomByte]

Great. Just to bring this up: We now check that there is no active lock before deleting files but we do not set a lock during the deletion

[d3xter666]

Thanks! I have missed that! Now, should be fine: f6e0404

[RandomByte]

What do we need the new manifest-*lock for?

[d3xter666]

Yep, these are missleading. I have changed them! Thanks!

c8c7c9d

[RandomByte]

Now they are renamed, but I'm still wondering what they are for 😅

I.e. what is their purpose compared to the locks we already have.

[d3xter666]

Ah, that’s fair 😅

Generally, they solve the issue with the race conditions.
What you have described above is just one if the cases! There are more.
And these locks solve exactly that.

So far, we had only one lock that was for “downloading” the artifacts. But in there’s also an Install phase that actually copies the files to the cache folder!
We have maven and npm install. I just wanted to distinguish them as they use different origins. Otherwise, they could easily be with the same name (I think so 🤔)

From that perspective, there were lots of options for a race conditions window:

• between cache clean and the confirmation window
• Between download and install phase of the packages

Before the latest change, at certain point, both could have continued.

Now, (all) locks are respected and even if there’s a little chance to jump between the phases, it will notice the lock for the other process and stop.

Hopefully, this makes it a bit clearer!

If you insist, I can make the locks with the same name/prefix i.e. ‘instal-*.lock’

[RandomByte]

I can't find this "phase that actually copies the files to the cache folder". Let's take a look together in a call

[matz3]

I think the usability of the command could be improved.

• No information about the actual filesystem paths of the cache and relevant dirs (not in CLI description, not in --help, not in command output
• No information about the relation to ui5DataDir config / UI5_DATA_DIR env var
• No output when command is executed, and it might take a while to calculate the size, so users might assume the command is stuck if it does not print anything for a minute or longer.
• Output lists two different sizes 197.0 MB vs 196.8 MB:

The following items from cache will be removed:
  • buildCache/v0_7 (197.0 MB)

Total: 197.0 MB


✓ Removed buildCache/v0_7 (196.8 MB)

Success: Cleaned 1 entry, freed 196.8 MB

• (minor) Inconsistency in logged paths (framework/ vs framework):

The following items from cache will be removed:
  • framework/ (711.4 MB)

Total: 711.4 MB

Do you want to continue? (y/N) y

✓ Removed framework (711.4 MB)

[d3xter666]

I have tried to improve the situation with the infmration for the execution of the cache clean command and have addressed all your concenrns.

Let me know if there's something more you'd expect.

Regarding the size report, there are some considerations we need to be aware of:

• Getting framework cache dir size might require time. Even if you do that on your machine, it will take time. Even the du command on Mac uses cache in order to be quick, but the first run is not that fast. The optimal solution: Give information about the number of files that will be deleted
• The purge of the DB cache is just the opposite! Selecting COUNT(*) of all rows might take significant amount of time to get the results. On the other hand, the size of the DB can be quite fast.

I have tried to somehow get advantage of these findings and provide the optimal solution- show metrics for what is fast and provide generic messages. Any other solutions will require certain compromises.

[d3xter666]

Edit

Now the UX of this command is revised!

The real issue is enormous cache!

Collecting full information about what would be deleted is a haevy work and we simply cannot do it real time. In the end, for the end user it's important what would be deleted!
Given that, my proposal would be a summary information that does not need to collect all the files or their sizes. Instead, just put an overview:

Confirmation

Checking cache at /my/data/dir/.ui5 …

The following cached data will be removed:

  • UI5 Framework packages   /my/data/dir/.ui5/framework   (1 project, 18 libraries, 212 versions)
  • Build cache (DB)         /my/data/dir/.ui5/buildCache/v0_7   (100.0 KB)

Do you want to continue? (y/N)

During cache cleanup

If we now decide to accept purging of the hughe cache, it will take some time cleaning it up. We cannot do anythinbg about it, but simply wait for the fuiles to be deleted.
For that purpose, I have created an real time "monitor" of what is being deleted:

 ⠏ UI5 Framework packages   …/sap.m/1.52.14/src/sap/m/BarRenderer.js   2.86 s

Final summary

✓ Removed UI5 Framework packages   (/my/data/dir/.ui5/framework · 1 project, 18 libraries, 212 versions)
✓ Removed Build cache (DB)   (/my/data/dir/.ui5/buildCache/v0_7 · 100.0 MB)

Success: Cleaned UI5 Framework packages and Build cache (DB)

Hopefully, this is good enough as UX and fast enough, so that developers get feedback what is actually happenning

[RandomByte]

That's better. On my system it's very fast now:

Checking cache at /Users/me/.ui5 …

The following cached data will be removed:

  • UI5 Framework packages   /Users/me/.ui5/framework   (2 projects, 155 libraries, 1189 versions)
  • Build cache (DB)         /Users/me/.ui5/buildCache/v0_7   (70.7 MB)

Do you want to continue? (y/N) 

But I think the term projects is misleading here. Usually every UI5 app or library is a project. I would drop that entirely and rather say 1.189 versions of 155 libraries (ideally with a decimal separator?)

I don't really think we need a progress indicator for the deletion process. It is expected to take a while, I would rather not add code just for that.

[matz3]

According to the docs of lockfile package:

All known locks are removed when the process exits.

So why do we need to close the server now? So far this has worked fine for us without the need to cleanly call close.

Also, those handlers are only registered for the CLI command, not for the API usage. In addition, we also listen to SIGHUP and SIGBREAK for the cleanup hooks in ProjectBuilder. I wonder why those have not been included here.

[matz3]

I think it would be worth spending the time to provide one API to calculate the data dir. We already had many places for this, and now there are even more..

[matz3]

I think there is a mixup of different parts of the caches.

This lock is set by the server, and my assumption is that it exists to prevent removal of cached framework libraries, and to prevent removal of the build cache, which both might be in use. There is a special case in which we actually could remove the cache (no framework deps + cache off), but I think it's okay to leave this special case out here and still abort.

So I think it should not belong to the framework cache, but to the overall cache that this command is cleaning. For this reason, I think the error message is incorrect.

[matz3]

I now receive this error when a server is running, but when I start a build, and then run the clean command, the build fails as files (framework / build cache) have been removed in between.