This repository has been archived by the owner on Mar 6, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 94
Resolve the location of subiquity_client without symlinks #610
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change makes use of the `package_config` Dart package to resolve the location of the `subiquity_client` because the symlinks to `subiquity_client/subiquity` and `.subiquity/socket` in the project tree are problematic in many ways. Not only we've had to duplicate the symlinks for both the desktop installer and the WSL setup, but any desktop installer flavor would have to create similar symlinks too, to be able to dry-run the installer flavor. Also, the symlinks are problematic for snap builds. The socket symlinks break local (as in source-type: local) builds. Using Git as the source-type circumvents this problem, but the drawback is that then snapcraft doesn't pick local uncommitted changes. Close: canonical#477
This looks good to me. Happy for this to be strengthen! I don’t see any immediate negative impact on WSL, but I would appreciate @CarlosNihelton to have a look as he is the one using it in a dev environment from day to day :) |
That's really great! Just tested on Ubuntu 22.04 on WSL2 and it works great in dry-run. We benefit from hardening the build environment. Great job! |
CarlosNihelton
approved these changes
Jan 18, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very, very minor nitpick. Otherwise, LGTM.
Co-authored-by: Carlos Nihelton <carlosnsoliveira@gmail.com>
CarlosNihelton
approved these changes
Jan 18, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
Thanks! |
jpnurmi
added a commit
to jpnurmi/ubuntu-desktop-installer
that referenced
this pull request
Jan 28, 2022
Integration tests were accidentally broken by canonical#610. The tests are no longer correctly locating the .subiquity directory. Exposing the lookup logic in SubiquityServer allows integration tests to locate .subiquity for verifying output files.
jpnurmi
added a commit
that referenced
this pull request
Feb 1, 2022
Integration tests were accidentally broken by #610. The tests are no longer correctly locating the .subiquity directory. Exposing the lookup logic in SubiquityServer allows integration tests to locate .subiquity for verifying output files.
jpnurmi
added a commit
to jpnurmi/ubuntu-desktop-installer
that referenced
this pull request
Mar 21, 2022
This is a continuation of canonical#610 and canonical#665. Using a relative path is not enough. For flavors, the path to subiquity's socket grows even longer, because it's located in Pub cache and includes a full sha1 in its name. $HOME/.pub-cache/git/ubuntu-desktop-installer-<sha1>/packages/subiquity_client/subiquity/.subiquity/socket
jpnurmi
added a commit
to jpnurmi/ubuntu-desktop-installer
that referenced
this pull request
Mar 21, 2022
This is a continuation of canonical#610 and canonical#665. Using a relative path is not enough. For flavors, the path to subiquity's socket grows even longer, because it's located in Pub cache and includes a full sha1 in its name. $HOME/.pub-cache/git/ubuntu-desktop-installer-<sha1>/packages/subiquity_client/subiquity/.subiquity/socket
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change makes use of the
package_config
Dart package to resolvethe location of the
subiquity_client
because the symlinks tosubiquity_client/subiquity
and.subiquity/socket
in the projecttree are problematic in many ways.
Not only we've had to duplicate the symlinks for both the desktop
installer and the WSL setup, but any desktop installer flavor would
have to create similar symlinks too, to be able to dry-run the
installer flavor.
Also, the symlinks are problematic for snap builds. The socket symlinks
break local (as in source-type: local) builds. Using Git as the
source-type circumvents this problem, but the drawback is that then
snapcraft doesn't pick local uncommitted changes. Lastly, the symlinks
break remote builds too.
Close: #477