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.

Sign up for GitHub

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

Remove checkoutLocation from StartWorkspaceReq #9007

Merged
merged 2 commits into from
Apr 7, 2022
Merged

Remove checkoutLocation from StartWorkspaceReq #9007

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b86d7f4
Remove checkoutLocation from StartWorkspaceReq
csweichel Mar 29, 2022
5cca449
Use checkout location during content init
Furisto Mar 31, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion components/content-service-api/go/blobs.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion components/content-service-api/go/content.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion components/content-service-api/go/headless-log.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion components/content-service-api/go/ideplugin.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

90 changes: 51 additions & 39 deletions components/content-service-api/go/initializer.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion components/content-service-api/go/workspace.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion components/content-service-api/initializer.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,9 @@ message PrebuildInitializer {
}

// FromBackupInitializer initializes content from a previously made backup
message FromBackupInitializer {}
message FromBackupInitializer {
string checkout_location = 1;
}

// GitStatus describes the current Git working copy status, akin to a combination of "git status" and "git branch"
message GitStatus {
Expand Down
3 changes: 3 additions & 0 deletions components/content-service-api/typescript/src/initializer_pb.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,8 @@ export namespace PrebuildInitializer {
}

export class FromBackupInitializer extends jspb.Message {
getCheckoutLocation(): string;
setCheckoutLocation(value: string): FromBackupInitializer;

serializeBinary(): Uint8Array;
toObject(includeInstance?: boolean): FromBackupInitializer.AsObject;
Expand All @@ -311,6 +313,7 @@ export class FromBackupInitializer extends jspb.Message {

export namespace FromBackupInitializer {
export type AsObject = {
checkoutLocation: string,
}
}

Expand Down
31 changes: 30 additions & 1 deletion components/content-service-api/typescript/src/initializer_pb.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2328,7 +2328,7 @@ proto.contentservice.FromBackupInitializer.prototype.toObject = function(opt_inc
*/
proto.contentservice.FromBackupInitializer.toObject = function(includeInstance, msg) {
var f, obj = {

checkoutLocation: jspb.Message.getFieldWithDefault(msg, 1, "")
};

if (includeInstance) {
Expand Down Expand Up @@ -2365,6 +2365,10 @@ proto.contentservice.FromBackupInitializer.deserializeBinaryFromReader = functio
}
var field = reader.getFieldNumber();
switch (field) {
case 1:
var value = /** @type {string} */ (reader.readString());
msg.setCheckoutLocation(value);
break;
default:
reader.skipField();
break;
Expand Down Expand Up @@ -2394,6 +2398,31 @@ proto.contentservice.FromBackupInitializer.prototype.serializeBinary = function(
*/
proto.contentservice.FromBackupInitializer.serializeBinaryToWriter = function(message, writer) {
var f = undefined;
f = message.getCheckoutLocation();
if (f.length > 0) {
writer.writeString(
1,
f
);
}
};


/**
* optional string checkout_location = 1;
* @return {string}
*/
proto.contentservice.FromBackupInitializer.prototype.getCheckoutLocation = function() {
return /** @type {string} */ (jspb.Message.getFieldWithDefault(this, 1, ""));
};


/**
* @param {string} value
* @return {!proto.contentservice.FromBackupInitializer} returns this
*/
proto.contentservice.FromBackupInitializer.prototype.setCheckoutLocation = function(value) {
return jspb.Message.setProto3StringField(this, 1, value);
};


Expand Down
19 changes: 19 additions & 0 deletions components/content-service/pkg/initializer/initializer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,3 +516,22 @@ func PlaceWorkspaceReadyFile(ctx context.Context, wspath string, initsrc csapi.W

return nil
}

func GetCheckoutLocationFromInitializer(init *csapi.WorkspaceInitializer) string {
switch {
case init.GetGit() != nil:
return init.GetGit().CheckoutLocation
case init.GetPrebuild() != nil && len(init.GetPrebuild().Git) > 0:
return init.GetPrebuild().Git[0].CheckoutLocation
case init.GetBackup() != nil:
return init.GetBackup().CheckoutLocation
case init.GetComposite() != nil:
for _, c := range init.GetComposite().Initializer {
loc := GetCheckoutLocationFromInitializer(c)
if loc != "" {
return loc
}
}
}
return ""
}
2 changes: 1 addition & 1 deletion components/gitpod-protocol/src/wsready.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* See License-AGPL.txt in the project root for license information.
*/

// generated using github.com/32leaves/bel on 2022-02-15 11:53:18.380158212 +0000 UTC m=+0.011913675
// generated using github.com/32leaves/bel on 2022-03-29 17:08:06.113873917 +0000 UTC m=+0.010035965
// DO NOT MODIFY

export enum WorkspaceInitSource {
Expand Down
1 change: 0 additions & 1 deletion components/image-builder-mk3/pkg/orchestrator/orchestrator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,6 @@ func (o *Orchestrator) Build(req *protocol.BuildRequest, resp protocol.ImageBuil
Owner: buildWorkspaceOwnerID,
},
Spec: &wsmanapi.StartWorkspaceSpec{
CheckoutLocation: ".",
Initializer: initializer,
Timeout: maxBuildRuntime.String(),
WorkspaceImage: o.Config.BuilderImage,
Expand Down
9 changes: 6 additions & 3 deletions components/server/src/workspace/workspace-starter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1175,7 +1175,6 @@ export class WorkspaceStarter {
}

const spec = new StartWorkspaceSpec();
spec.setCheckoutLocation(checkoutLocation!);
await createGitpodTokenPromise;
spec.setEnvvarsList(envvars);
spec.setGit(this.createGitSpec(workspace, user));
Expand All @@ -1188,7 +1187,7 @@ export class WorkspaceStarter {
spec.setIdeImage(startWorkspaceSpecIDEImage);
spec.setDeprecatedIdeImage(ideImage);
spec.setWorkspaceImage(instance.workspaceImage);
spec.setWorkspaceLocation(workspace.config.workspaceLocation || spec.getCheckoutLocation());
spec.setWorkspaceLocation(workspace.config.workspaceLocation || checkoutLocation);
spec.setFeatureFlagsList(this.toWorkspaceFeatureFlags(featureFlags));
if (workspace.type === "regular") {
spec.setTimeout(this.userService.workspaceTimeoutToDuration(await userTimeoutPromise));
Expand Down Expand Up @@ -1317,7 +1316,11 @@ export class WorkspaceStarter {
const disp = new DisposableCollection();

if (mustHaveBackup) {
result.setBackup(new FromBackupInitializer());
const backup = new FromBackupInitializer();
if (CommitContext.is(context)) {
backup.setCheckoutLocation(context.checkoutLocation || "");
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we default to "."? : 🤔

Copy link
Member

Choose a reason for hiding this comment

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

Being picky here bc we had an incident after the last change in the multirepo PR

Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we default to "."?

Also in all cases, even if not a CommitContext?

Copy link
Member

Choose a reason for hiding this comment

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

@csweichel Could you double-check here?

Copy link
Member

Choose a reason for hiding this comment

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

@AlexTugarev This could be the problematic lines, actually. ☝️

}
result.setBackup(backup);
} else if (SnapshotContext.is(context)) {
const snapshot = new SnapshotInitializer();
snapshot.setSnapshot(context.snapshotBucketId);
Expand Down
20 changes: 1 addition & 19 deletions components/ws-daemon/pkg/content/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ func (s *WorkspaceService) creator(req *api.InitWorkspaceRequest) session.Worksp
return func(ctx context.Context, location string) (res *session.Workspace, err error) {
return &session.Workspace{
Location: location,
CheckoutLocation: getCheckoutLocation(req),
CheckoutLocation: wsinit.GetCheckoutLocationFromInitializer(req.Initializer),
CreatedAt: time.Now(),
Owner: req.Metadata.Owner,
WorkspaceID: req.Metadata.MetaId,
Expand All @@ -273,24 +273,6 @@ func ServiceDirName(instanceID string) string {
return instanceID + "-daemon"
}

// getCheckoutLocation returns the first checkout location found of any Git initializer configured by this request
func getCheckoutLocation(req *api.InitWorkspaceRequest) string {
spec := req.Initializer.Spec
if ir, ok := spec.(*csapi.WorkspaceInitializer_Git); ok {
if ir.Git != nil {
return ir.Git.CheckoutLocation
}
}
if ir, ok := spec.(*csapi.WorkspaceInitializer_Prebuild); ok {
if ir.Prebuild != nil {
if len(ir.Prebuild.Git) > 0 {
return ir.Prebuild.Git[0].CheckoutLocation
}
}
}
return ""
}

// DisposeWorkspace cleans up a workspace, possibly after taking a final backup
func (s *WorkspaceService) DisposeWorkspace(ctx context.Context, req *api.DisposeWorkspaceRequest) (resp *api.DisposeWorkspaceResponse, err error) {
//nolint:ineffassign
Expand Down
2 changes: 1 addition & 1 deletion components/ws-daemon/pkg/internal/session/workspace.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ func (s *Workspace) UpdateGitStatus(ctx context.Context) (res *csapi.GitStatus,

loc = filepath.Join(loc, s.CheckoutLocation)
if !git.IsWorkingCopy(loc) {
log.WithField("loc", loc).WithFields(s.OWI()).Debug("did not find a Git working copy - not updating Git status")
log.WithField("loc", loc).WithField("checkout location", s.CheckoutLocation).WithFields(s.OWI()).Debug("did not find a Git working copy - not updating Git status")
return nil, nil
}

Expand Down
2 changes: 1 addition & 1 deletion components/ws-manager-api/core.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ message StartWorkspaceSpec {
repeated EnvironmentVariable envvars = 6;

// checkout_location describes where the code has been checked out to
string checkout_location = 7;
reserved 7;

// workspace_location describes where the workspace root of Theia will be
string workspace_location = 8;
Expand Down
Loading