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

meta: omit LD_LIBRARY_PATH and PATH in classic confinement #4216

Merged
merged 4 commits into from Jun 20, 2023

Conversation

mr-cal
Copy link
Collaborator

@mr-cal mr-cal commented Jun 16, 2023

  • Have you followed the guidelines for contributing?
  • Have you signed the CLA?
  • Have you successfully run make lint?
  • Have you successfully run pytest tests/unit?

When confinement is classic, do not use the default values for LD_LIBRARY_PATH and PATH.

Includes an optimization/refactor suggestion from my original implementation: #3758 (comment)

Resolves #4187

(CRAFT-1774)

Signed-off-by: Callahan Kovacs <callahan.kovacs@canonical.com>
@mr-cal mr-cal requested a review from cmatsuoka June 16, 2023 16:59
Copy link
Contributor

@cmatsuoka cmatsuoka left a comment

Choose a reason for hiding this comment

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

Thanks!

@codecov-commenter
Copy link

codecov-commenter commented Jun 19, 2023

Codecov Report

Merging #4216 (1039432) into main (64c8f1e) will decrease coverage by 0.02%.
The diff coverage is 72.41%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             main    #4216      +/-   ##
==========================================
- Coverage   93.90%   93.88%   -0.02%     
==========================================
  Files         569      569              
  Lines       43746    43763      +17     
==========================================
+ Hits        41078    41089      +11     
- Misses       2668     2674       +6     
Impacted Files Coverage Δ
tests/unit/meta/test_snap_yaml.py 96.75% <63.15%> (-3.25%) ⬇️
snapcraft/meta/snap_yaml.py 97.03% <90.00%> (+0.39%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@mr-cal mr-cal enabled auto-merge (squash) June 19, 2023 22:43
@mr-cal mr-cal merged commit 9b6e583 into main Jun 20, 2023
13 checks passed
@mr-cal mr-cal deleted the classic-environment branch June 20, 2023 00:26
Saviq added a commit to Miriway/Miriway that referenced this pull request Aug 28, 2023
Need to use snapcraft/edge for now:

canonical/snapcraft#4216
Saviq added a commit to canonical/mir that referenced this pull request Aug 28, 2023
Otherwise the environment is b0rked:
canonical/snapcraft#4216
Saviq added a commit to canonical/mir that referenced this pull request Aug 28, 2023
Otherwise the environment is b0rked:
canonical/snapcraft#4216
github-actions bot pushed a commit to Miriway/Miriway that referenced this pull request Aug 30, 2023
Need to use snapcraft/edge for now:

canonical/snapcraft#4216
@AlanGriffiths
Copy link

I realise this has merged, but this seems the best place to call out problems. Many of the classic snaps I maintain rely on having executables on $PATH (typically $SNAP/bin or $SNAP/usr/bin).

I doubt that I'm the only one using the existing behaviour of adding these to $PATH. While I have sympathy for the change dropping them (my snaps need logic to strip them) this is probably going to break a lot of snaps if promoted in the current form.

github-actions bot pushed a commit to Miriway/Miriway that referenced this pull request Oct 18, 2023
Need to use snapcraft/edge for now:

canonical/snapcraft#4216
github-actions bot pushed a commit to Miriway/Miriway that referenced this pull request Oct 18, 2023
Need to use snapcraft/edge for now:

canonical/snapcraft#4216
github-actions bot pushed a commit to Miriway/Miriway that referenced this pull request Oct 21, 2023
Need to use snapcraft/edge for now:

canonical/snapcraft#4216
github-actions bot pushed a commit to Miriway/Miriway that referenced this pull request Oct 30, 2023
Need to use snapcraft/edge for now:

canonical/snapcraft#4216
github-actions bot pushed a commit to Miriway/Miriway that referenced this pull request Nov 2, 2023
Need to use snapcraft/edge for now:

canonical/snapcraft#4216
github-actions bot pushed a commit to Miriway/Miriway that referenced this pull request Nov 2, 2023
Need to use snapcraft/edge for now:

canonical/snapcraft#4216
github-actions bot pushed a commit to Miriway/Miriway that referenced this pull request Nov 5, 2023
Need to use snapcraft/edge for now:

canonical/snapcraft#4216
github-actions bot pushed a commit to Miriway/Miriway that referenced this pull request Nov 17, 2023
Need to use snapcraft/edge for now:

canonical/snapcraft#4216
github-actions bot pushed a commit to Miriway/Miriway that referenced this pull request Nov 22, 2023
Need to use snapcraft/edge for now:

canonical/snapcraft#4216
AlanGriffiths pushed a commit to Miriway/Miriway that referenced this pull request Nov 30, 2023
Need to use snapcraft/edge for now:

canonical/snapcraft#4216
github-actions bot pushed a commit to Miriway/Miriway that referenced this pull request Dec 15, 2023
Need to use snapcraft/edge for now:

canonical/snapcraft#4216
github-actions bot pushed a commit to Miriway/Miriway that referenced this pull request Jan 1, 2024
Need to use snapcraft/edge for now:

canonical/snapcraft#4216
github-actions bot pushed a commit to Miriway/Miriway that referenced this pull request Jan 11, 2024
Need to use snapcraft/edge for now:

canonical/snapcraft#4216
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.

Environment is being set for classic snaps
4 participants