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

Fix DUB dependencies #11798

Closed
wants to merge 3 commits into from
Closed

Fix DUB dependencies #11798

wants to merge 3 commits into from

Conversation

rtbo
Copy link
Contributor

@rtbo rtbo commented May 18, 2023

Depending on DUB version, it will look either in the old cache structure or use the new DUB cache database (see dlang/dub#2642)

Fixes #11773

@rtbo rtbo requested a review from jpakkane as a code owner May 18, 2023 19:26
mesonbuild/dependencies/dub.py Outdated Show resolved Hide resolved
mesonbuild/dependencies/dub.py Outdated Show resolved Hide resolved
mesonbuild/dependencies/dub.py Outdated Show resolved Hide resolved
Comment on lines +555 to +593
def _find_cache_dir(self) -> str:
dubhome = self._find_dub_home()
# TODO: handle (rare) case where dubhome is overriden
# in Dub's settings file.
return os.path.join(dubhome, 'cache')
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need two logical functions for this? Just have one function, but add 'cache' to every return site?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is to make easier a future change to take into account the user customization of dubhome location in the settings (JSON files that Dub reads in various places)

Copy link
Member

Choose a reason for hiding this comment

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

Does that just change what the dubhome is while leaving the contents such as cache/ alone?

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 user can customize the location of dubhome in its personal settings.
The cache root is always [dubhome]/cache.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not mandatory for Meson to handle this. In case of dependency not found, I can add to the error message the cache location we're searching in to indicate to the user that Meson will not try other cache location

mesonbuild/dependencies/dub.py Outdated Show resolved Hide resolved
mesonbuild/dependencies/dub.py Outdated Show resolved Hide resolved
mesonbuild/modules/dlang.py Outdated Show resolved Hide resolved
@@ -1,12 +1,15 @@
project('dub-example', 'd')

error('MESON_SKIP_TEST: Dub support is broken at the moment (#11773)')
Copy link
Member

Choose a reason for hiding this comment

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

Hold on, why was this ever unconditionally errored out in the first place???

We should never do that. Dub support wasn't broken until after the version is checked for the version that introduced breakage.

Disabling tests on all versions of Dub means we get zero integration tests for anything at all, even the commits that introduced conditional errors.

mesonbuild/dependencies/dub.py Outdated Show resolved Hide resolved
@rtbo rtbo force-pushed the dub_cache_db branch 2 times, most recently from 41b0451 to b13afaa Compare May 19, 2023 09:35
@rtbo
Copy link
Contributor Author

rtbo commented May 19, 2023

I squashed everything (except dlang module fix and tests reactivation).
Let me know if it's OK.
EDIT: still a little debug to do...

@rtbo rtbo force-pushed the dub_cache_db branch 3 times, most recently from 7d6d6b5 to aa48333 Compare May 20, 2023 15:08
@rtbo
Copy link
Contributor Author

rtbo commented May 20, 2023

MacOS failures are unrelated (LLVM framework).

@rtbo rtbo marked this pull request as draft May 21, 2023 16:24
@rtbo
Copy link
Contributor Author

rtbo commented May 21, 2023

I put this on hold for a while. Possibly we can enrich dub describe and revert the cache database PR in Dub.

@rtbo
Copy link
Contributor Author

rtbo commented Jun 4, 2023

According this comment in dlang/dub#2644, it appears that the DUB cache database will stay.
As the cache DB is merged and the other proposal seems to stall, I would propose to keep this PR as is.

@rtbo rtbo marked this pull request as ready for review June 4, 2023 16:35
@rtbo
Copy link
Contributor Author

rtbo commented Jun 5, 2023

DUB 1.33.0 was just released, but the cache database is not in yet.
It will be in 1.34.0.

rtbo added 3 commits July 22, 2023 18:51
Depending on DUB version, it will look either in the old cache structure
or use the new DUB cache database
Was broken because of DubDependency.class_dubbin typing change.
Passed under radar because Dub tests were deactivated in the same PR.
@jpakkane
Copy link
Member

Test failure seems unrelated.

@rtbo
Copy link
Contributor Author

rtbo commented Aug 8, 2023

Hi, anything blocking from merging this PR?

rtbo added a commit to rtbo/squiz-box that referenced this pull request Aug 8, 2023
Right now Dub support in meson is broken.
See mesonbuild/meson#11798
@rtbo rtbo closed this Aug 6, 2024
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.

OpenSUSE CI image builder fails to run dub tests
3 participants