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

Set <pkgname_datadir> to an absolute path #10830

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

mpickering
Copy link
Collaborator

Ticket #10717 points out that the _datadir environment variable
should be set to an absolute path since the test executables may
change directory during their invocation.

This is easily fixed by using the absoluteWorkingDirLBI
function, which was introduced in 027bddf,

Fixes #10717


Template Α: This PR modifies behaviour or interface

Include the following checklist in your PR:

Ticket #10717 points out that the <pkgname>_datadir environment variable
should be set to an absolute path since the test executables may
change directory during their invocation.

This is easily fixed by using the absoluteWorkingDirLBI
function, which was introduced in 027bddf,

Fixes #10717
@mpickering mpickering requested a review from sheaf March 14, 2025 13:05
The test checks that if you change directory when running a test
executable that you can still access datafiles.
@Mikolaj
Copy link
Member

Mikolaj commented Mar 17, 2025

I take it you deem this ready for review, so let me add the label for that.

Comment on lines 932 to +934
addInternalBuildTools
:: PackageDescription
:: AbsolutePath (Dir Pkg)
-> PackageDescription
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a Cabal API change. We can't backport it. Is that right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It would be possible to backport by adding a new function instead of modifying the type of the existing one.

@mpickering
Copy link
Collaborator Author

@ulysses4ever I have put up a backport on #10838. I don't plan to make any more changes to the backport, so the person preparing the release can decide what a suitable course of action is and modify that patch or this patch as they deem necessary.

@ulysses4ever
Copy link
Collaborator

Sounds good, thanks @mpickering !

@mpickering mpickering added the merge me Tell Mergify Bot to merge label Mar 27, 2025
@mergify mergify bot added the ready and waiting Mergify is waiting out the cooldown period label Mar 27, 2025
@Mikolaj
Copy link
Member

Mikolaj commented Mar 27, 2025

Apparently, conflicts prevent a merge.

BTW, should this be backported to 3.14?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Tell Mergify Bot to merge ready and waiting Mergify is waiting out the cooldown period regression in 3.14
Projects
None yet
5 participants