Skip to content

Conversation

@MarcelCoding
Copy link
Member

Motivation

I've also implemented the changes from #14364 for the progress bar now.

Although it works, I am not quite happy with it.

If I copy something big from one host to another, the total size will probably be >1GiB. The problem now is that the current progress stays as 0 for way too long because the already transferred bytes are still below 0.1GiB.

2025-10-30_09-54

and after like 10 seconds it looks like that:

2025-10-30_09-59

This problem could be fixed by not displaying the unit only once for a X/X/X GiB block, but instead for every block: X GiB/ X GiB/X GiB.

In the reality, this could look like this: 100KiB/25MiB/1GiB. But I am afraid this could be too verbose.

Maybe the community or maintainers could express some kind of opinion in the direction this should be going.

Also, this could just be a me-problem, because I am expecting that the number is always changing because it was like that till now.

Context


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@xokdvium
Copy link
Contributor

In the reality, this could look like this: 100KiB/25MiB/1GiB

I like this in general. If the units are the same we could skip intermediate units though and keep the last one. That would make things less verbose and reduce duplication.

@MarcelCoding
Copy link
Member Author

MarcelCoding commented Nov 1, 2025

@xokdvium I've just implemented your suggestion:

2025-11-01_18-15 2025-11-01_18-18

I kind of dislike that it is still kind too quiet. How about adding some kind of spinner like with npm or pnpm that indicates that nix is not frozen and still working?

@xokdvium
Copy link
Contributor

xokdvium commented Nov 1, 2025

kind of spinner like with npm or pnpm

Sounds great! That could be a separate PR if you'd like.

throw UsageError("'%s' is not an integer", s);
}

size_t getSizeUnit(int64_t value);
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we maybe add an enum for the unit? Something like enum class SizeUnit

Copy link
Member Author

Choose a reason for hiding this comment

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

like its implemented now?

@MarcelCoding MarcelCoding force-pushed the progress-bar-units branch 2 times, most recently from 35e1301 to ba9fd9b Compare November 2, 2025 02:10
@MarcelCoding MarcelCoding force-pushed the progress-bar-units branch 2 times, most recently from 5b07a5d to db9a922 Compare November 2, 2025 14:49
Comment on lines 103 to 104
#define SIZE_UNITS \
X(Base, 'K') \
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#define SIZE_UNITS \
X(Base, 'K') \
#define NIX_UTIL_SIZE_UNITS \
X(Base, 'K') \

Copy link
Contributor

Choose a reason for hiding this comment

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

This is more generous to the out-of-tree users of nix headers. It would also be a good idea to undefine it at the end.

Copy link
Member Author

@MarcelCoding MarcelCoding Nov 2, 2025

Choose a reason for hiding this comment

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

The problem is that it is not directly clear to me where the undefine should be. Because this define is also used in the util.cc file and not only in the util.hh one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants