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

change: never ignore promote until-clean under -p #8721

Closed
wants to merge 4 commits into from

Conversation

emillon
Copy link
Collaborator

@emillon emillon commented Sep 21, 2023

This extends #5956 to all versions of (lang dune), since this is a property of the CLI.

This extends ocaml#5956 to all versions of `(lang dune)`, since this is a
property of the CLI.

Signed-off-by: Etienne Millon <[email protected]>
@rgrinberg
Copy link
Member

Isn't this exactly what I've tried here? #8512 It didn't work because it indeed broke stuff downstream.

Instead of filtering, I thought we agreed to turn them into fallback instead.

@emillon
Copy link
Collaborator Author

emillon commented Sep 21, 2023

Ah yes I was not sure if you meant to make these rules fallback too. I just updated the PR but there's a test failing:

;; More complex test

(rule
 (target into+ignoring)
 (mode (promote (into subdir)))
 (action (with-stdout-to %{target} (echo "hello"))))

(rule
 (target into+ignoring)
 (enabled_if %{ignoring_promoted_rules})
 (action (copy subdir/into+ignoring into+ignoring)))

I think it's invalid now, but it's probably a smaller repro of some existing package. WDYT?

@rgrinberg
Copy link
Member

I think it's invalid now, but it's probably a smaller repro of some existing package. WDYT?

Isn't this basically a manually created fallback rule?

@rgrinberg
Copy link
Member

In any case, given that this variable exists (though it seems to be used) I feel like this change is big enough that perhaps we should save this all until 4.0?

@emillon
Copy link
Collaborator Author

emillon commented Sep 21, 2023

Agree. Let's keep #8706 though for 3.11.

@rgrinberg
Copy link
Member

What's the rush to get it out for 3.11? The bug fix is basically incomplete so this fix helps nobody still. Moreover, it introduces a confusing behavior

@emillon
Copy link
Collaborator Author

emillon commented Sep 21, 2023

What about generated opam files that now fail because of #8518? Do we call that an intended breakage?

@rgrinberg
Copy link
Member

I'm reverting that as well.

@emillon emillon closed this Sep 26, 2023
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.

2 participants