Skip to content

Comments

Revert "Adapt scheduler to work with dynamic derivations"#9081

Merged
Ericson2314 merged 1 commit intoNixOS:masterfrom
obsidiansystems:hacky-fix-9052
Oct 2, 2023
Merged

Revert "Adapt scheduler to work with dynamic derivations"#9081
Ericson2314 merged 1 commit intoNixOS:masterfrom
obsidiansystems:hacky-fix-9052

Conversation

@Ericson2314
Copy link
Member

Motivation

This reverts commit 5e3986f. This un-implements RFC 92 but fixes the critical bug #9052 which many people are hitting. This is a decent stop-gap until a minimal reproduction of that bug is found and a proper fix can be made.

Context

Mostly fixed #9052, but I would like to leave that issue open until we have a regression test, so I can then properly fix the bug (unbreaking RFC 92) later.

Priorities

Add 👍 to pull requests you find important.

@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Oct 2, 2023
@Ericson2314 Ericson2314 force-pushed the hacky-fix-9052 branch 3 times, most recently from 5148a4a to d85a08c Compare October 2, 2023 03:40
This reverts commit 5e3986f. This
un-implements RFC 92 but fixes the critical bug NixOS#9052 which many people
are hitting. This is a decent stop-gap until a minimal reproduction of
that bug is found and a proper fix can be made.

Mostly fixed NixOS#9052, but I would like to leave that issue open until we
have a regression test, so I can then properly fix the bug (unbreaking
RFC 92) later.
@github-actions
Copy link

github-actions bot commented Oct 2, 2023

Successfully created backport PR for 2.18-maintenance:

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-10-02-nix-team-meeting-minutes-91/33775/1

Ericson2314 added a commit that referenced this pull request Nov 20, 2023
…ns""

This fixes dynamic derivations, reverting #9081. #9052 However will be
reintroduced unless this is modified somehow.

This reverts commit 8440afb.
@fricklerhandwerk fricklerhandwerk added bug store Issues and pull requests concerning the Nix store labels Jan 21, 2024
@github-actions
Copy link

Backport failed for 2.18-maintenance, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin 2.18-maintenance
git worktree add -d .worktree/backport-9081-to-2.18-maintenance origin/2.18-maintenance
cd .worktree/backport-9081-to-2.18-maintenance
git switch --create backport-9081-to-2.18-maintenance
git cherry-pick -x 8440afbed756254784d9fea3eaab06649dffd390

1 similar comment
@github-actions
Copy link

Backport failed for 2.18-maintenance, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin 2.18-maintenance
git worktree add -d .worktree/backport-9081-to-2.18-maintenance origin/2.18-maintenance
cd .worktree/backport-9081-to-2.18-maintenance
git switch --create backport-9081-to-2.18-maintenance
git cherry-pick -x 8440afbed756254784d9fea3eaab06649dffd390

@Ericson2314
Copy link
Member Author

(manual backport has been done)

Ericson2314 added a commit that referenced this pull request Mar 22, 2024
This fixes dynamic derivations, reverting #9081. #9052 However will be
reintroduced unless this is modified somehow.

This reverts commit 8440afb.
Ericson2314 added a commit that referenced this pull request Oct 11, 2024
This fixes dynamic derivations, reverting #9081. #9052 However will be
reintroduced unless this is modified somehow.

This reverts commit 8440afb.
Ericson2314 added a commit that referenced this pull request Feb 1, 2025
This fixes dynamic derivations, reverting #9081. #9052 However will be
reintroduced unless this is modified somehow.

This reverts commit 8440afb.
Ericson2314 added a commit that referenced this pull request Feb 2, 2025
This fixes dynamic derivations, reverting #9081.

I believe that this time around, #9052 is fixed. When I first rebased
this, tests were failing (which wasn't the case before). The cause of
those test failures were due to the crude job in which the outer goal
tried to exit with the inner goal's status.

Now, that error handling has been reworked to be more faithful. The exit
exit status and exception of the inner goal is returned by the outer
goal. The exception was what was causing the test failures, but I
believe it was not having the right error code (there is more than one
for failure) that caused #9081.

The only cost of doing things the "right way" was that I had to
introduce a hacky `preserveException` boolean. I don't like this, but,
then again, none of us like anything about how the scheduler works.
Issue #11927 is still there to clean everything up, subsuming the need
for any `preserveException` because I doubt we will be fishing
information out of state machines like this at all.

This reverts commit 8440afb.
Ericson2314 added a commit that referenced this pull request Feb 2, 2025
This fixes dynamic derivations, reverting #9081.

I believe that this time around, #9052 is fixed. When I first rebased
this, tests were failing (which wasn't the case before). The cause of
those test failures were due to the crude job in which the outer goal
tried to exit with the inner goal's status.

Now, that error handling has been reworked to be more faithful. The exit
exit status and exception of the inner goal is returned by the outer
goal. The exception was what was causing the test failures, but I
believe it was not having the right error code (there is more than one
for failure) that caused #9081.

The only cost of doing things the "right way" was that I had to
introduce a hacky `preserveException` boolean. I don't like this, but,
then again, none of us like anything about how the scheduler works.
Issue #11927 is still there to clean everything up, subsuming the need
for any `preserveException` because I doubt we will be fishing
information out of state machines like this at all.

This reverts commit 8440afb.
Ericson2314 added a commit that referenced this pull request Feb 2, 2025
This fixes dynamic derivations, reverting #9081.

I believe that this time around, #9052 is fixed. When I first rebased
this, tests were failing (which wasn't the case before). The cause of
those test failures were due to the crude job in which the outer goal
tried to exit with the inner goal's status.

Now, that error handling has been reworked to be more faithful. The exit
exit status and exception of the inner goal is returned by the outer
goal. The exception was what was causing the test failures, but I
believe it was not having the right error code (there is more than one
for failure) that caused #9081.

The only cost of doing things the "right way" was that I had to
introduce a hacky `preserveException` boolean. I don't like this, but,
then again, none of us like anything about how the scheduler works.
Issue #11927 is still there to clean everything up, subsuming the need
for any `preserveException` because I doubt we will be fishing
information out of state machines like this at all.

This reverts commit 8440afb.
Ericson2314 added a commit that referenced this pull request Feb 4, 2025
This fixes dynamic derivations, reverting #9081.

I believe that this time around, #9052 is fixed. When I first rebased
this, tests were failing (which wasn't the case before). The cause of
those test failures were due to the crude job in which the outer goal
tried to exit with the inner goal's status.

Now, that error handling has been reworked to be more faithful. The exit
exit status and exception of the inner goal is returned by the outer
goal. The exception was what was causing the test failures, but I
believe it was not having the right error code (there is more than one
for failure) that caused #9081.

The only cost of doing things the "right way" was that I had to
introduce a hacky `preserveException` boolean. I don't like this, but,
then again, none of us like anything about how the scheduler works.
Issue #11927 is still there to clean everything up, subsuming the need
for any `preserveException` because I doubt we will be fishing
information out of state machines like this at all.

This reverts commit 8440afb.
Ericson2314 added a commit that referenced this pull request Feb 5, 2025
This fixes dynamic derivations, reverting #9081.

I believe that this time around, #9052 is fixed. When I first rebased
this, tests were failing (which wasn't the case before). The cause of
those test failures were due to the crude job in which the outer goal
tried to exit with the inner goal's status.

Now, that error handling has been reworked to be more faithful. The exit
exit status and exception of the inner goal is returned by the outer
goal. The exception was what was causing the test failures, but I
believe it was not having the right error code (there is more than one
for failure) that caused #9081.

The only cost of doing things the "right way" was that I had to
introduce a hacky `preserveException` boolean. I don't like this, but,
then again, none of us like anything about how the scheduler works.
Issue #11927 is still there to clean everything up, subsuming the need
for any `preserveException` because I doubt we will be fishing
information out of state machines like this at all.

This reverts commit 8440afb.

Co-Authored-By: Eelco Dolstra <[email protected]>
Ericson2314 added a commit that referenced this pull request Feb 5, 2025
This fixes dynamic derivations, reverting #9081.

I believe that this time around, #9052 is fixed. When I first rebased
this, tests were failing (which wasn't the case before). The cause of
those test failures were due to the crude job in which the outer goal
tried to exit with the inner goal's status.

Now, that error handling has been reworked to be more faithful. The exit
exit status and exception of the inner goal is returned by the outer
goal. The exception was what was causing the test failures, but I
believe it was not having the right error code (there is more than one
for failure) that caused #9081.

The only cost of doing things the "right way" was that I had to
introduce a hacky `preserveException` boolean. I don't like this, but,
then again, none of us like anything about how the scheduler works.
Issue #11927 is still there to clean everything up, subsuming the need
for any `preserveException` because I doubt we will be fishing
information out of state machines like this at all.

This reverts commit 8440afb.

Co-Authored-By: Eelco Dolstra <[email protected]>
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nix-dynamic-derivations-a-practical-application/61541/6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug store Issues and pull requests concerning the Nix store with-tests Issues related to testing. PRs with tests have some priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

error: path is not valid using nix ==2.18.0

4 participants