lpereira / hardinfo

System profiler and benchmark tool for Linux systems
http://hardinfo.org
GNU General Public License v2.0
774 stars 130 forks source link

modules/benchmark: Results in english, then retranslated in client #630

Closed ocerman closed 3 years ago

ocerman commented 3 years ago

This PR force set English locale when reading the machine information for benchmarks. Then the necessary strings are re-translated when constructing shell string. FIX #556

The English locale is set by setting the LANGUAGE env. variable and resetting the LC_ALL. Seems to work fine on all my machines, but probably needs more testing whether it works reliably in all environments (BSD?)

lpereira commented 3 years ago

While this helps for now -- and is part of the solution, although I'd work directly in the database and fix it there rather than modifying the results locally -- I don't think it's the right fix for #556.

I should've been more explicit in the issue, but the idea is that the information should be stored in a machine-parsable format. Columns in the database in the server side of things, and interfacing with the client storing this in a JSON object per result.

So, instead of having something like

"1 physical processor; 4 cores; 8 threads"

We'd have this stored in a struct:

struct {
      int num_procs;
      int num_cores;
      int num_threads;
};

And store/load that in the JSON that the server operates with.

On Sat, Nov 6, 2021 at 6:28 PM Ondrej Čerman @.***> wrote:

This PR force set English locale when reading the machine information for benchmarks. Then the necessary strings are re-translated when constructing shell string. FIX #556 https://github.com/lpereira/hardinfo/issues/556

The English locale is set by setting the LANGUAGE env. variable and resetting the LC_ALL. Seems to work fine on all my machines, but probably needs more testing whether it works reliably in all environments (BSD?)

You can view, comment on, or merge this pull request online at:

https://github.com/lpereira/hardinfo/pull/630 Commit Summary

File Changes

(2 files https://github.com/lpereira/hardinfo/pull/630/files)

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/lpereira/hardinfo/pull/630, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAADVGLJ7EHDH5UX2T6TZBTUKVXTDANCNFSM5HP3ZQEA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

-- Leandro

ocerman commented 3 years ago

@lpereira yeah, that's probably better and more future-proof. I will work on it.

lpereira commented 3 years ago

So, that information is already stored in the server the way we need them; we can probably just stop sending the string altogether and focus on the structured data: https://github.com/lpereira/hardinfo/blob/server/main.go#L37-L40 ; thinking about just ignoring CpuConfig coming from HardInfo and generating one from these variables in HardInfo itself.

On Sat, Nov 6, 2021 at 6:28 PM Ondrej Čerman @.***> wrote:

This PR force set English locale when reading the machine information for benchmarks. Then the necessary strings are re-translated when constructing shell string. FIX #556 https://github.com/lpereira/hardinfo/issues/556

The English locale is set by setting the LANGUAGE env. variable and resetting the LC_ALL. Seems to work fine on all my machines, but probably needs more testing whether it works reliably in all environments (BSD?)

You can view, comment on, or merge this pull request online at:

https://github.com/lpereira/hardinfo/pull/630 Commit Summary

File Changes

(2 files https://github.com/lpereira/hardinfo/pull/630/files)

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/lpereira/hardinfo/pull/630, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAADVGLJ7EHDH5UX2T6TZBTUKVXTDANCNFSM5HP3ZQEA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

-- Leandro

lpereira commented 3 years ago

OK, so I modified the server to generate a CpuDesc in English for now (so that previous versions of HardInfo still have something to show). What we need to do is ignore that and build the string on the fly now with the information we have.

On Sat, Nov 6, 2021 at 10:37 PM Leandro A. F. Pereira @.***> wrote:

So, that information is already stored in the server the way we need them; we can probably just stop sending the string altogether and focus on the structured data: https://github.com/lpereira/hardinfo/blob/server/main.go#L37-L40 ; thinking about just ignoring CpuConfig coming from HardInfo and generating one from these variables in HardInfo itself.

On Sat, Nov 6, 2021 at 6:28 PM Ondrej Čerman @.***> wrote:

This PR force set English locale when reading the machine information for benchmarks. Then the necessary strings are re-translated when constructing shell string. FIX #556 https://github.com/lpereira/hardinfo/issues/556

The English locale is set by setting the LANGUAGE env. variable and resetting the LC_ALL. Seems to work fine on all my machines, but probably needs more testing whether it works reliably in all environments (BSD?)

You can view, comment on, or merge this pull request online at:

https://github.com/lpereira/hardinfo/pull/630 Commit Summary

File Changes

(2 files https://github.com/lpereira/hardinfo/pull/630/files)

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/lpereira/hardinfo/pull/630, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAADVGLJ7EHDH5UX2T6TZBTUKVXTDANCNFSM5HP3ZQEA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

-- Leandro

-- Leandro

lpereira commented 3 years ago

Modified the server once again so that CpuConfig is now parsed from the old format (kind of what you did there, but server-side), and stored in JSON on the server. The value is then built back as the format used by HardInfo (for current versions of the client), and in a CpuConfigMap map that maps frequency and number of cores in that frequency, so that we have structured data to rebuild the string using the user's locale (including how they spell MHz, if they use dot or comma for the decimal separator, etc, etc).

On Sat, Nov 6, 2021 at 11:15 PM Leandro A. F. Pereira @.***> wrote:

OK, so I modified the server to generate a CpuDesc in English for now (so that previous versions of HardInfo still have something to show). What we need to do is ignore that and build the string on the fly now with the information we have.

On Sat, Nov 6, 2021 at 10:37 PM Leandro A. F. Pereira @.***> wrote:

So, that information is already stored in the server the way we need them; we can probably just stop sending the string altogether and focus on the structured data: https://github.com/lpereira/hardinfo/blob/server/main.go#L37-L40 ; thinking about just ignoring CpuConfig coming from HardInfo and generating one from these variables in HardInfo itself.

On Sat, Nov 6, 2021 at 6:28 PM Ondrej Čerman @.***> wrote:

This PR force set English locale when reading the machine information for benchmarks. Then the necessary strings are re-translated when constructing shell string. FIX #556 https://github.com/lpereira/hardinfo/issues/556

The English locale is set by setting the LANGUAGE env. variable and resetting the LC_ALL. Seems to work fine on all my machines, but probably needs more testing whether it works reliably in all environments (BSD?)

You can view, comment on, or merge this pull request online at:

https://github.com/lpereira/hardinfo/pull/630 Commit Summary

File Changes

(2 files https://github.com/lpereira/hardinfo/pull/630/files)

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/lpereira/hardinfo/pull/630, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAADVGLJ7EHDH5UX2T6TZBTUKVXTDANCNFSM5HP3ZQEA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

-- Leandro

-- Leandro

-- Leandro

lpereira commented 3 years ago

Client should be using structured data for both CPU description and config now.

On Mon, Nov 8, 2021 at 2:34 AM Leandro A. F. Pereira @.***> wrote:

Modified the server once again so that CpuConfig is now parsed from the old format (kind of what you did there, but server-side), and stored in JSON on the server. The value is then built back as the format used by HardInfo (for current versions of the client), and in a CpuConfigMap map that maps frequency and number of cores in that frequency, so that we have structured data to rebuild the string using the user's locale (including how they spell MHz, if they use dot or comma for the decimal separator, etc, etc).

On Sat, Nov 6, 2021 at 11:15 PM Leandro A. F. Pereira @.***> wrote:

OK, so I modified the server to generate a CpuDesc in English for now (so that previous versions of HardInfo still have something to show). What we need to do is ignore that and build the string on the fly now with the information we have.

On Sat, Nov 6, 2021 at 10:37 PM Leandro A. F. Pereira @.***> wrote:

So, that information is already stored in the server the way we need them; we can probably just stop sending the string altogether and focus on the structured data: https://github.com/lpereira/hardinfo/blob/server/main.go#L37-L40 ; thinking about just ignoring CpuConfig coming from HardInfo and generating one from these variables in HardInfo itself.

On Sat, Nov 6, 2021 at 6:28 PM Ondrej Čerman @.***> wrote:

This PR force set English locale when reading the machine information for benchmarks. Then the necessary strings are re-translated when constructing shell string. FIX #556 https://github.com/lpereira/hardinfo/issues/556

The English locale is set by setting the LANGUAGE env. variable and resetting the LC_ALL. Seems to work fine on all my machines, but probably needs more testing whether it works reliably in all environments (BSD?)

You can view, comment on, or merge this pull request online at:

https://github.com/lpereira/hardinfo/pull/630 Commit Summary

File Changes

(2 files https://github.com/lpereira/hardinfo/pull/630/files)

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/lpereira/hardinfo/pull/630, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAADVGLJ7EHDH5UX2T6TZBTUKVXTDANCNFSM5HP3ZQEA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

-- Leandro

-- Leandro

-- Leandro

-- Leandro