Skip to content

Remove unused ignore parameter of extends() directive#35588

Merged
tgamblin merged 1 commit intospack:developfrom
adamjstewart:deprecate/extends-ignore
Mar 20, 2023
Merged

Remove unused ignore parameter of extends() directive#35588
tgamblin merged 1 commit intospack:developfrom
adamjstewart:deprecate/extends-ignore

Conversation

@adamjstewart
Copy link
Copy Markdown
Member

@adamjstewart adamjstewart commented Feb 20, 2023

If I'm understanding correctly, the ignore parameter was only used for spack activate/deactivate, it isn't used by Spack Environments which has its own handling of file conflicts. We should remove it.

Is there somewhere I can check for this and issue a deprecation warning? Or is it harmlessly ignored anyway? This is one of those situations where we use kwargs where we really shouldn't, but have to keep doing so due to old Python version support.

@spackbot-app spackbot-app bot added core PR affects Spack core functionality documentation Improvements or additions to documentation extends python update-package labels Feb 20, 2023
@haampie
Copy link
Copy Markdown
Member

haampie commented Feb 20, 2023 •
edited
Loading

In environment views we ignore file-file conflicts right now.

Not sure if this argument is still used in spack view ... to make file-file conflicts non-fatal.

@adamjstewart
Copy link
Copy Markdown
Member Author

adamjstewart commented Feb 24, 2023

@spackbot run pipeline

@spackbot-app
Copy link
Copy Markdown

spackbot-app bot commented Feb 24, 2023

I've started that pipeline for you!

@adamjstewart
Copy link
Copy Markdown
Member Author

adamjstewart commented Feb 24, 2023

@spackbot run pipeline

@spackbot-app
Copy link
Copy Markdown

spackbot-app bot commented Feb 24, 2023

I've started that pipeline for you!

@alalazo alalazo added this to the v0.20.0 milestone Mar 1, 2023
@adamjstewart
Copy link
Copy Markdown
Member Author

adamjstewart commented Mar 2, 2023

@spackbot run pipeline

@spackbot-app
Copy link
Copy Markdown

spackbot-app bot commented Mar 2, 2023

I've started that pipeline for you!

@adamjstewart
Copy link
Copy Markdown
Member Author

adamjstewart commented Mar 2, 2023

@spackbot run pipeline

@spackbot-app
Copy link
Copy Markdown

spackbot-app bot commented Mar 2, 2023

I've started that pipeline for you!

@adamjstewart adamjstewart force-pushed the deprecate/extends-ignore branch 2 times, most recently from a8d47c8 to 973e9bd Compare March 8, 2023 18:10
@adamjstewart adamjstewart force-pushed the deprecate/extends-ignore branch from 973e9bd to 489e59a Compare March 11, 2023 17:10
@tgamblin tgamblin self-requested a review March 19, 2023 02:05
@adamjstewart adamjstewart force-pushed the deprecate/extends-ignore branch from 489e59a to 18e622b Compare March 20, 2023 04:56
@spack spack deleted a comment from spackbot-app bot Mar 20, 2023
@tgamblin
Copy link
Copy Markdown
Member

tgamblin commented Mar 20, 2023

Everything that handles ignore= was removed in #29317 and included in 0.19, when we removed spack activate and spack deactivate in favor of environments. So all of these are already being ignored by Spack -- the only place they were used was in activate/deactivate.

@tgamblin tgamblin changed the title Deprecate ignore parameter of extends directive Remove usages of unused ignore parameter of extends() directive Mar 20, 2023
@tgamblin tgamblin changed the title Remove usages of unused ignore parameter of extends() directive Remove unused ignore parameter of extends() directive Mar 20, 2023
@tgamblin tgamblin enabled auto-merge (squash) March 20, 2023 06:15
@tgamblin
Copy link
Copy Markdown
Member

tgamblin commented Mar 20, 2023

@adamjstewart I just enabled auto-mere on this since ignore is harmlessly ignored by Spack. I think we should get the change in before it needs another rebase.

I'll put in a follow-up PR to remove the unused extendee_args() method and to restructure the way extends() works. I'll also add a deprecation warning for ignore= specifically and stop accepting other arguments to extends.

@alalazo
Copy link
Copy Markdown
Member

alalazo commented Mar 20, 2023

@spackbot run pipeline

@spackbot-app
Copy link
Copy Markdown

spackbot-app bot commented Mar 20, 2023

I've started that pipeline for you!

@tgamblin tgamblin merged commit 5dc8ed2 into spack:develop Mar 20, 2023
@tgamblin
Copy link
Copy Markdown
Member

tgamblin commented Mar 20, 2023

@alalazo how did the pipeline fail?

@alalazo
Copy link
Copy Markdown
Member

alalazo commented Mar 20, 2023

It was a spurious failure with a "Spack error" on a Python package. On rebuild the package was installed correctly.

@alalazo
Copy link
Copy Markdown
Member

alalazo commented Mar 20, 2023

The python package in question is py-protobuf, here's the rebuild that went ✔️ : https://gitlab.spack.io/spack/spack/-/pipelines/306861

@adamjstewart adamjstewart deleted the deprecate/extends-ignore branch March 20, 2023 15:09
@tgamblin
Copy link
Copy Markdown
Member

tgamblin commented Mar 20, 2023 •
edited
Loading

do you remember what type of spack error or can you link to the broken pipeline?

@alalazo
Copy link
Copy Markdown
Member

alalazo commented Mar 20, 2023

This is the pipeline that was failing: https://gitlab.spack.io/spack/spack/-/pipelines/306620 And this is the job in question: https://gitlab.spack.io/spack/spack/-/jobs/6182731

@alalazo
Copy link
Copy Markdown
Member

alalazo commented Mar 20, 2023

It doesn't give a lot of hints other than "Failed to call Python: exit status -4"

@tgamblin
Copy link
Copy Markdown
Member

tgamblin commented Mar 20, 2023

Yeah and the log is just the tty output showing the command line that was executed. This one's weird.

tgamblin added a commit that referenced this pull request Oct 2, 2023
@tgamblin
extensions: remove unused extendee_args() method
62eed3d 
This method is vestigial; the only arg we ever used was `ignore=`, and that was
eliminated in #29317 and #35588.

The `kwargs` field of the extensions dictionary is actually completely unused now. Add a
note for future removal.
tgamblin added a commit that referenced this pull request Oct 30, 2023
@tgamblin
extensions: remove unused extendee_args() method
fa17dcc 
This method is vestigial; the only arg we ever used was `ignore=`, and that was
eliminated in #29317 and #35588.

The `kwargs` field of the extensions dictionary is actually completely unused now. Add a
note for future removal.
tgamblin added a commit that referenced this pull request Nov 10, 2023
@tgamblin
extensions: remove unused extendee_args() method
cc38978 
This method is vestigial; the only arg we ever used was `ignore=`, and that was
eliminated in #29317 and #35588.

The `kwargs` field of the extensions dictionary is actually completely unused now. Add a
note for future removal.
tgamblin added a commit that referenced this pull request Jan 2, 2024
@tgamblin
extensions: remove unused extendee_args() method
2d9e9c2 
This method is vestigial; the only arg we ever used was `ignore=`, and that was
eliminated in #29317 and #35588.

The `kwargs` field of the extensions dictionary is actually completely unused now. Add a
note for future removal.
tgamblin added a commit that referenced this pull request Jan 3, 2024
@tgamblin
extensions: remove unused extendee_args() method
d36333f 
This method is vestigial; the only arg we ever used was `ignore=`, and that was
eliminated in #29317 and #35588.

The `kwargs` field of the extensions dictionary is actually completely unused now. Add a
note for future removal.
@tgamblin tgamblin mentioned this pull request Jan 3, 2024
Merged
tgamblin added a commit that referenced this pull request Jan 3, 2024
@tgamblin
extensions: remove unused extendee_args() method
7381c57 
This method is vestigial; the only arg we ever used was `ignore=`, and that was
eliminated in #29317 and #35588.

The `kwargs` field of the extensions dictionary is actually completely unused now. Add a
note for future removal.
alalazo pushed a commit that referenced this pull request Jan 4, 2024
@tgamblin
extensions: remove unused extendee_args() method (#41942)
c46f3cf 
This method is vestigial; the only arg we ever used was `ignore=`, and that was
eliminated in #29317 and #35588.

The `kwargs` field of the extensions dictionary is actually completely unused now. Add a
note for future removal.
RikkiButler20 pushed a commit to RikkiButler20/spack that referenced this pull request Jan 25, 2024
@tgamblin @RikkiButler20
extensions: remove unused extendee_args() method (spack#41942)
82aec28 
This method is vestigial; the only arg we ever used was `ignore=`, and that was
eliminated in spack#29317 and spack#35588.

The `kwargs` field of the extensions dictionary is actually completely unused now. Add a
note for future removal.
RikkiButler20 pushed a commit to RikkiButler20/spack that referenced this pull request Jan 31, 2024
@tgamblin @RikkiButler20
extensions: remove unused extendee_args() method (spack#41942)
6d3a273 
This method is vestigial; the only arg we ever used was `ignore=`, and that was
eliminated in spack#29317 and spack#35588.

The `kwargs` field of the extensions dictionary is actually completely unused now. Add a
note for future removal.
kwryankrattiger pushed a commit to kwryankrattiger/spack that referenced this pull request Jul 9, 2024
@tgamblin @kwryankrattiger
extensions: remove unused extendee_args() method (spack#41942)
f4d07f3 
This method is vestigial; the only arg we ever used was `ignore=`, and that was
eliminated in spack#29317 and spack#35588.

The `kwargs` field of the extensions dictionary is actually completely unused now. Add a
note for future removal.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tgamblin tgamblin tgamblin approved these changes

@pradyunsg pradyunsg Awaiting requested review from pradyunsg

@thomas-bouvier thomas-bouvier Awaiting requested review from thomas-bouvier

@scheibelp scheibelp Awaiting requested review from scheibelp

@haampie haampie Awaiting requested review from haampie

@becker33 becker33 Awaiting requested review from becker33

Assignees

No one assigned

Labels

core PR affects Spack core functionality documentation Improvements or additions to documentation extends python update-package

Projects

None yet

Milestone

v0.20.0

Development

Successfully merging this pull request may close these issues.

4 participants

@adamjstewart @haampie @tgamblin @alalazo