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.

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

Fluster "installed" version #145

Merged
merged 4 commits into from
Nov 26, 2023
Merged

Fluster "installed" version #145

merged 4 commits into from
Nov 26, 2023

Conversation

rgonzalezfluendo
Copy link
Contributor

  • Use tempfile.gettempdir() for temporal results directory:

    • before commit results_dir is __file__ /../results
    • after commit results_dir is /tmp/fluster_results

    The global argument output can be used for custom temporal results dir.

  • Using platform dirs to execute installed fluster.

    Checking if fluster is uninstalled with the existence of the '.git' dir.

    For installed fluster, the site platform dirs is preferred if exists. If not, user platform dirs will be used, that is, ~/.local/share/fluster (or XDG_DATA_HOME) will be used if /usr/share/fluster (or XDG_DATA_DIRS) doesn't exist.

    Fluster tries to use the platformdirs[1] lib. But if not found, it fallbacks to a simple implementation. The reason is to avoid introducing the first external dependency.

    [1] https://github.com/platformdirs/platformdirs

Copy link
Contributor

@obbardc obbardc left a comment

Choose a reason for hiding this comment

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

Just a couple of thoughts.

line = "=" * 100
print(
f"{line}\n"
f'WARNING: Using "{TEST_SUITES_DIR_SYS}" with fluster uninstalled.\n'
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe it would be worth just printing the test suite directory without the warning when running from a local directory ? This would then be useful for people running fluster locally.

I'm just not sure how useful the warning text really is :-).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The goal with installed/uninstalled flavor is to deprecate and delete code added in a22f609

If a sys dir want to be used, the installed version must be used.

fluster/main.py Outdated Show resolved Hide resolved
fluster/main.py Outdated Show resolved Hide resolved
fluster/main.py Outdated Show resolved Hide resolved
fluster/fluster.py Outdated Show resolved Hide resolved
fluster/main.py Outdated Show resolved Hide resolved
fluster/platformdirs.py Outdated Show resolved Hide resolved
fluster/platformdirs.py Outdated Show resolved Hide resolved
fluster/main.py Outdated Show resolved Hide resolved
fluster/platformdirs.py Outdated Show resolved Hide resolved
fluster/main.py Outdated Show resolved Hide resolved
fluster/main.py Outdated Show resolved Hide resolved
fluster/main.py Outdated Show resolved Hide resolved
fluster/main.py Show resolved Hide resolved
@rgonzalezfluendo
Copy link
Contributor Author

rgonzalezfluendo commented Nov 15, 2023

TODO when approved:

  • rebase fixups
  • Update commit-msg to drop text about platformdirs lib.

fluster/platformdirs.py Outdated Show resolved Hide resolved
@rgonzalezfluendo rgonzalezfluendo force-pushed the platformdirs branch 4 times, most recently from 414e603 to fe9e74e Compare November 26, 2023 22:44
use_emoji=not args.no_emoji,
verbose=args.verbose if "verbose" in args else False,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ylatuya @mdimopoulos

Verbose should be a global argument. But this change breaks the BS.

rgonzalezfluendo and others added 4 commits November 27, 2023 00:01
 * before commit results_dir is `__file__ /../results`
 * after commit results_dir is `/tmp/fluster_results`

The global arguments `output` can be used for custom temporal results dir.
Checking if fluster is uninstalled with the existence of the '.git'
dir.

For installed fluster, the site platform dirs is preferred for test
suites dir if exists. If not, user platform dirs will be used, that
is, ~/.local/share/fluster (or XDG_DATA_HOME) will be used if
/usr/share/fluster (or XDG_DATA_DIRS) doesn't exist. For resources dir
the user platform dirs is always used.

For uninstalled fluster, the usage of platform test suites dir [2] is
deprecated and it will be deleted in the future.

The platformdirs[1] lib was not used on purpose. The reason is to
avoid introducing the first external non-dev dependency.

[1] https://github.com/platformdirs/platformdirs
[2] a22f609
Needed to print internal direcotries.
@rgonzalezfluendo rgonzalezfluendo merged commit 2e8d5ab into master Nov 26, 2023
2 checks passed
@rgonzalezfluendo rgonzalezfluendo deleted the platformdirs branch November 26, 2023 23:07
rgonzalezfluendo pushed a commit that referenced this pull request Nov 27, 2023
With the last PR [1], the Debian package stopped working. Fluster was checking
first path in XDG_DATA_DIRS instead of iterate over them.

Debian package install the test suites in `/usr/share/fluster/test_suites/`. And a normal value of XDG_DATA_DIRS is
`/home/user/.local/share/flatpak/exports/share:/var/lib/flatpak/exports/share:/usr/local/share/:/usr/share/`

Fixes #157

[1] #145
rgonzalezfluendo pushed a commit that referenced this pull request Nov 27, 2023
With the last PR [1], the Debian package stopped working. Fluster was checking
first path in XDG_DATA_DIRS instead of iterate over them.

Debian package install the test suites in `/usr/share/fluster/test_suites/`. And a normal value of XDG_DATA_DIRS is
`/home/user/.local/share/flatpak/exports/share:/var/lib/flatpak/exports/share:/usr/local/share/:/usr/share/`

Fixes #157

[1] #145
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.

4 participants