Add Default Machine Setting #383

technic960183 · 2024-12-05T15:14:45Z

Summary

This PR introduces a mechanism to specify default machine settings at both the repository and user levels, enhancing flexibility and usability in configuring the --machine argument for configure.py.

Rationale

This design mirrors common configuration practices used by tools such as git, vim, and yt, offering flexibility with multiple levels of configuration. The repository-level setting provides a tailored environment for specific projects (as @ChunYen-Chen ask for), while the user-level setting allows global defaults (as @hyschive ask for).

Features Added

Two Levels of Default Settings:

Repository-Level (.local_settings): Located in src/.local_settings to specify default settings for a specific repository.
User-Level (global_settings): Stored in $HOME/.config/gamer/global_settings, serving as a global fallback when repository-level settings are absent.

In this PR, there is only one setting machine in the default setting file. But this feature is capable of adding more settings easily.

Note: The name of the setting files are not .gamerrc because this is not a "rc" (runtime configuration) for GAMER.

Backward compatibility

The default machine is determined in the following order:

Explicit argument passed to configure.py.
Repository-level local settings.
User-level global settings.
Fallback to eureka_intel.

Files Changed

Changes in `src/configure.py`

Major

Add class SystemSetting.
If --machine is not given, it will look for sys_setting.get_default() which store the setting from the default setting files.
Raises a FileNotFoundError if the .config files are not found.

Minor

Add a validation section to check the Python version.
Some styles update.
Remove _get_option_tuples overridden. And add allow_abbrev=False to ArgumentParser.
Replace a general try-except with a specify condition checking.

Add Setting Script `tool/config/set_settings.sh`

A new script tool/set_settings.sh is provided to help users set the default setting (default machine here) during the initial setup.

Update related Wiki pages

Installation: Add guide to set the default machine and remove --machine related words.
Installation: Machine Configuration File: Improve the reading flow.
Installation: Option List: Add description about the default machine.
Quick Start: Add guide to set the default machine and improve the reading flow.
Quick Start: 1D Shock Tube: Remove --machine related words.

Removal of Hardcoded Defaults in `generate_make.sh`

Removed hardcoded --machine=eureka_intel in these scripts, ensuring that the new hierarchical default mechanism is utilized.

Updated `.gitignore`

Added src/.local_settings to .gitignore to avoid tracking repository-level configuration files.

Tests

Follow the docs to set up the default machine.
Follow the Quick Start guide to test if the flow works.
Test if configure.py --machine= override the setting files and works if the settings files are not set.
Test if fallback to eureka_intel works.
Test if the global setting works with a new cloned repo.
Copy Wiki to the forked repo for a preview.
Test the behavior of multiple settings (for future extension. Disable in this PR for now.)

Other Major Change

Check the version of Python. Require Python 3.5 or later to run configure.py.
Python 3.4 has ended its life nearly 6 years ago (since 2019-03-18). It will be fine for us to not support Python 3.4 and lower.
Ref: https://devguide.python.org/versions/

Add a feature that read and write `.local_settings` (repository level) and `global_settings` (user level) files to save the default machine. Add tool/set_settings.sh script to setup the default machine. In src/configure.py: - Change the default of `--machine` to None. - Add function `get_machine()` to read the default machine setting. - Raise FileNotFoundError if the .config file is not found. Add `src/.local_settings` to .gitignore.

They will fall back to `eureka_intel` when not specified. If not removed, `--machine=eureka_intel` will override the user setting.

technic960183 · 2024-12-12T09:40:15Z

Rebasing and force-pushing updated for including change of #376 .
Will wait for #373 to merge for updating the Wiki.

Fix `get_default()` in `SystemSetting`. Replace except with non-explicit exception. Remove `_`, an alias from gettext module, which is not used.

Give ability to add more parameters and add delete option. All done by Copilot O1-preview!

Add options verbose, list, and clear-all. Some changes with a better cli flow Use PADDED_KEYS for better alignment. Accept short options to be combined.

ChunYen-Chen

@technic960183 Thanks for the contribution. I have left some comments. There are some typos and a bug that need to be fixed. The rest of the comments are optimization and improving maintainability. Let me know if you have better ideas, or we can discuss it at the lab.

src/configure.py

tool/set_settings.sh

doc/wiki/Installation:-Machine-Configuration-File.md

tool/set_settings.sh

doc/wiki/Installation:-Option-List.md

Co-authored-by: ChunYen-Chen <chunyenchen2019@gmail.com>

- Use global variable to store the return value of a function. - Redirect error messages to stderr.

- Improve usage messages - Make `-v` and `-l` works the same - Remove `realpath` for macOS compatibility

For supporting the use of type annotations

tool/config/set_settings.sh

src/configure.py

tool/config/set_settings.sh

doc/wiki/Quick-Start.md

doc/wiki/Installation-related/Installation:-Option-List.md

doc/wiki/Installation.md

src/configure.py

hyschive · 2025-01-15T08:49:50Z

@technic960183 Please resolve the conflicts.

Co-authored-by: Chun-Yen Chen <70311975+ChunYen-Chen@users.noreply.github.com>

`_get_option_tuples` was overriden before to disable abbreviation for Python 3.4 and below. The parameter `allow_abbrev` was added in Python 3.5. Now, since in this pull request gamer-project#383 we ask for Python 3.5 or above, we can remove the override and set `allow_abbrev` to `False`.

technic960183

Thanks for the detailed comments.
I have fixed some problems and leave 2 questions (which I have tagged you).
I do not agree with some of the suggestion and have commented the reasons. Please tell me what you think. Thanks.

doc/wiki/Installation.md

tool/config/set_settings.sh

doc/wiki/Installation.md

doc/wiki/Quick-Start.md

tool/config/set_settings.sh

Links: - Installation:-Option-List#--machine - Installation#default_setting

doc/wiki/Installation-related/Installation:-Option-List.md

src/configure.py

technic960183

Updated. Thanks for the review and suggestions.

src/configure.py

doc/wiki/Installation-related/Installation:-Option-List.md

hyschive

@technic960183 This PR looks great and works well. Thanks for the contribution! I've added some comments before merging. Besides, here is a general question:

Currently if src/.local_settings exists, even if it's empty, we completely ignore ~/.config/gamer/global_settings. I want to confirm if this behavior is intentional. For example, if both setting files exist, but machine is specified only in global_settings but not .local_settings, should we
- Adopt the default value eureka_intel, as specified in configure.py, or
- Adopt the value set in global_settings?

doc/wiki/Installation-related/Installation:-Machine-Configuration-File.md

doc/wiki/Installation-related/Installation:-Option-List.md

doc/wiki/Installation.md

src/configure.py

Docs related suggestion Co-authored-by: Hsi-Yu Schive <hyschive@gmail.com>

Co-authored-by: Hsi-Yu Schive <hyschive@gmail.com>

The empty local setting will not overwrite the global setting now.

technic960183

I have applied the suggestion and fixed the bug.
And the PR description is completely updated with a new testing tasks list.

@hyschive
It is a bug that if the .local_settings is empty, global_settings is ignored. I have fixed it and thanks for the catch.
I have also fixed the bug if there are more than one avaiable setting exist. Now each setting will override independently.

@ChunYen-Chen
I have removed load_setting() and put the functionality into the class SystemSetting. Please help me to review it. Thanks!

doc/wiki/Installation.md

doc/wiki/Installation-related/Installation:-Option-List.md

src/configure.py

ChunYen-Chen

@technic960183 Thanks for the updates and the bug fixes. Please fix the minor style comments. Sorry for not testing the PR fully after the update of the set_setting.sh.

src/configure.py

Co-authored-by: Chun-Yen Chen <70311975+ChunYen-Chen@users.noreply.github.com>

hyschive

@technic960183 Works great! Thanks for the contribution!

technic960183 marked this pull request as draft December 5, 2024 15:20

technic960183 added 2 commits December 12, 2024 17:15

Remove --machine=eureka_intel

b0573ce

They will fall back to `eureka_intel` when not specified. If not removed, `--machine=eureka_intel` will override the user setting.

technic960183 force-pushed the add_default_machine branch from 0c417fe to b0573ce Compare December 12, 2024 09:26

ChunYen-Chen and others added 4 commits December 13, 2024 21:29

Generalize system setting and update code style

adddaf5

Fix bug and imporve the flow of the code.

3f22d24

Fix `get_default()` in `SystemSetting`. Replace except with non-explicit exception. Remove `_`, an alias from gettext module, which is not used.

Generalized set_settings.sh

3df4413

Give ability to add more parameters and add delete option. All done by Copilot O1-preview!

Add new options and improve the cli

7fd4994

Add options verbose, list, and clear-all. Some changes with a better cli flow Use PADDED_KEYS for better alignment. Accept short options to be combined.

technic960183 force-pushed the add_default_machine branch from 30d0910 to 7fd4994 Compare December 16, 2024 10:36

technic960183 added 3 commits December 18, 2024 18:01

Remove option for testing

691e9eb

Merge branch 'main' into add_default_machine

103af05

Update wiki for setting default machine

209ea75

technic960183 marked this pull request as ready for review December 26, 2024 18:08

hyschive requested a review from ChunYen-Chen December 27, 2024 01:26

hyschive assigned ChunYen-Chen Dec 27, 2024

hyschive added enhancement compilation labels Dec 27, 2024

ChunYen-Chen requested changes Dec 27, 2024

View reviewed changes

technic960183 and others added 11 commits January 1, 2025 00:13

Apply easy suggestions from the reviewer

095cd84

Co-authored-by: ChunYen-Chen <chunyenchen2019@gmail.com>

Correct the return value of a bash function

afdb160

- Use global variable to store the return value of a function. - Redirect error messages to stderr.

Remove verbose mode output

2fe9a1d

- Improve usage messages - Make `-v` and `-l` works the same - Remove `realpath` for macOS compatibility

Add print_key and remove PADDED_KEYS

3ab32b5

Fix overwriting in the read-only action

bf4c196

Organize the code with comments chunks

a160c1a

Put checking to section VALIDATE PARAMETERS

c7ca863

Move set_settings.sh from tool/ to tool/config/

8302b8b

Merge branch 'main' into add_default_machine

5dc80f7

Update wiki for a better reading flow

1207766

Add python version check to configure.py

9de77ac

For supporting the use of type annotations

ChunYen-Chen requested changes Jan 14, 2025

View reviewed changes

technic960183 and others added 4 commits January 15, 2025 18:38

Apply easy suggestions from code review

af7fa6e

Co-authored-by: Chun-Yen Chen <70311975+ChunYen-Chen@users.noreply.github.com>

Completely remove --verbose and -v options

4bad5b3

Rename the local variables to all lowercase

6ccdb02

technic960183 commented Jan 15, 2025

View reviewed changes

Merge branch 'main' into add_default_machine

435d58e

technic960183 force-pushed the add_default_machine branch from a7366e3 to 435d58e Compare January 16, 2025 07:36

Add links between wiki pages

3e885b7

Links: - Installation:-Option-List#--machine - Installation#default_setting

ChunYen-Chen reviewed Jan 16, 2025

View reviewed changes

doc/wiki/Installation-related/Installation:-Option-List.md Outdated Show resolved Hide resolved

ChunYen-Chen reviewed Jan 16, 2025

View reviewed changes

src/configure.py Outdated Show resolved Hide resolved

technic960183 commented Jan 17, 2025

View reviewed changes

src/configure.py Outdated Show resolved Hide resolved

doc/wiki/Installation-related/Installation:-Option-List.md Outdated Show resolved Hide resolved

Update wiki and style

9d30e5d

technic960183 force-pushed the add_default_machine branch from 680579c to 9d30e5d Compare January 17, 2025 05:52

ChunYen-Chen approved these changes Jan 17, 2025

View reviewed changes

hyschive requested changes Jan 20, 2025

View reviewed changes

technic960183 and others added 3 commits January 20, 2025 14:38

Apply suggestions from code review

dfe756f

Docs related suggestion Co-authored-by: Hsi-Yu Schive <hyschive@gmail.com>

Fix wiki link and indentation

4cfbca9

Co-authored-by: Hsi-Yu Schive <hyschive@gmail.com>

Fix overwriting problem of the default setting

7e39dcd

The empty local setting will not overwrite the global setting now.

technic960183 commented Jan 20, 2025

View reviewed changes

doc/wiki/Installation.md Outdated Show resolved Hide resolved

doc/wiki/Installation-related/Installation:-Option-List.md Outdated Show resolved Hide resolved

src/configure.py Outdated Show resolved Hide resolved

src/configure.py Outdated Show resolved Hide resolved

technic960183 requested a review from ChunYen-Chen January 20, 2025 13:16

ChunYen-Chen approved these changes Jan 21, 2025

View reviewed changes

src/configure.py Outdated Show resolved Hide resolved

src/configure.py Outdated Show resolved Hide resolved

src/configure.py Outdated Show resolved Hide resolved

src/configure.py Outdated Show resolved Hide resolved

technic960183 requested a review from hyschive January 21, 2025 03:41

style applied from code review

948ed71

Co-authored-by: Chun-Yen Chen <70311975+ChunYen-Chen@users.noreply.github.com>

technic960183 force-pushed the add_default_machine branch from e3cca1d to 948ed71 Compare January 21, 2025 03:55

style

833f44a

Co-authored-by: Chun-Yen Chen <70311975+ChunYen-Chen@users.noreply.github.com>

hyschive approved these changes Jan 21, 2025

View reviewed changes

hyschive merged commit 693be16 into gamer-project:main Jan 21, 2025

technic960183 deleted the add_default_machine branch January 21, 2025 11:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Default Machine Setting #383

Add Default Machine Setting #383

technic960183 commented Dec 5, 2024 •

edited

Loading

technic960183 commented Dec 12, 2024 •

edited

Loading

ChunYen-Chen left a comment

hyschive commented Jan 15, 2025

technic960183 left a comment

technic960183 left a comment

hyschive left a comment •

edited

Loading

technic960183 left a comment •

edited

Loading

ChunYen-Chen left a comment

hyschive left a comment

Add Default Machine Setting #383

Add Default Machine Setting #383

Conversation

technic960183 commented Dec 5, 2024 • edited Loading

Summary

Rationale

Features Added

Backward compatibility

Files Changed

Changes in src/configure.py

Major

Minor

Add Setting Script tool/config/set_settings.sh

Update related Wiki pages

Removal of Hardcoded Defaults in generate_make.sh

Updated .gitignore

Tests

Other Major Change

technic960183 commented Dec 12, 2024 • edited Loading

ChunYen-Chen left a comment

Choose a reason for hiding this comment

hyschive commented Jan 15, 2025

technic960183 left a comment

Choose a reason for hiding this comment

technic960183 left a comment

Choose a reason for hiding this comment

hyschive left a comment • edited Loading

Choose a reason for hiding this comment

technic960183 left a comment • edited Loading

Choose a reason for hiding this comment

ChunYen-Chen left a comment

Choose a reason for hiding this comment

hyschive left a comment

Choose a reason for hiding this comment

technic960183 commented Dec 5, 2024 •

edited

Loading

Changes in `src/configure.py`

Add Setting Script `tool/config/set_settings.sh`

Removal of Hardcoded Defaults in `generate_make.sh`

Updated `.gitignore`

technic960183 commented Dec 12, 2024 •

edited

Loading

hyschive left a comment •

edited

Loading

technic960183 left a comment •

edited

Loading