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.

Sign up for GitHub

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

Jump to bottom

FromHumanSize accepts MiB but returns MB #31

Open
smoser opened this issue Feb 12, 2019 · 2 comments · May be fixed by #37
Open

FromHumanSize accepts MiB but returns MB #31

smoser opened this issue Feb 12, 2019 · 2 comments · May be fixed by #37

Comments

@smoser
Copy link

smoser commented Feb 12, 2019

One might expect that FromHumanSize("1MiB") would either

  • return error
  • return units.MiB (1024 * 1024 * 1024)

Currently it does not raise error and returns units.MB (1000 * 1000 * 1000)
@smoser
Copy link
Author

smoser commented Feb 12, 2019

units-fromhumansize.go.txt

cfergeau added a commit to cfergeau/go-units that referenced this issue Dec 15, 2020
@cfergeau
Add FromSize method
d403ec6 
go-units API is missing a method which would parse "32kB" as 32000
bytes, and "32kiB" as 32768 bytes.
FromHumanSize() parses both as 32000 bytes, while RAMInBytes() parses
both as 32768 bytes.

This commit introduces a FromSize method a more litteral parsing of the
unit is needed. Modifiers without a unit ('32k') will be assumed to be
using a decimal unit, so they'll equivalent to 32 kB/32000 bytes.

This fixes docker#31
@cfergeau cfergeau linked a pull request Dec 15, 2020 that will close this issue
Open
cfergeau added a commit to cfergeau/go-units that referenced this issue Dec 15, 2020
@cfergeau
Add FromSize method
dc6d94f 
go-units API is missing a method which would parse "32kB" as 32000
bytes, and "32kiB" as 32768 bytes.
FromHumanSize() parses both as 32000 bytes, while RAMInBytes() parses
both as 32768 bytes.

This commit introduces a FromSize method a more litteral parsing of the
unit is needed. Modifiers without a unit ('32k') will be assumed to be
using a decimal unit, so they'll equivalent to 32 kB/32000 bytes.

This fixes docker#31
@kolyshkin
Copy link
Contributor

Yes, this is the way it works. I think we can fix this and accept e.g. KB as "human size" (1000) and KiB as "computer size" (1024), defaulting to "human size" for "FromHumanSize" and to "computer size" for "RAMInBytes".

Problem is, this will make things slightly incompatible with the current behavior.

jadams pushed a commit to jadams/kali-vm that referenced this issue Sep 30, 2022
@elboulangero
Use GB rather than GiB for size
ed934aa 
It's not exactly a nitpick. debos uses github.com/docker/go-units to
parse the size, and docker/go-units is fundamentally broken, as it
parses both GiB anb GB as GB. It's a known issue that's been there
forever, and will probably never be fixed because backward compat
(that's my guess).

Cf. <docker/go-units#31>, or better
<docker/go-units#27 (comment)>.

So let's make sure that we talk about GB, because that's what happens in
practice.
jadams pushed a commit to jadams/kali-vm that referenced this issue Sep 30, 2022
@elboulangero
Use GB rather than GiB for size
bb1c955 
It's not exactly a nitpick. debos uses github.com/docker/go-units to
parse the size, and docker/go-units is fundamentally broken, as it
parses both GiB anb GB as GB. It's a known issue that's been there
forever, and will probably never be fixed because backward compat
(that's my guess).

Cf. <docker/go-units#31>, or better
<docker/go-units#27 (comment)>.

So let's make sure that we talk about GB, because that's what happens in
practice.
@grschafer grschafer mentioned this issue Oct 3, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add FromSize method cfergeau/go-units
2 participants
@smoser @kolyshkin