New issue
Advanced search Search tips

Issue 824948 link

Starred by 0 users
Status: Available
Owner: ----
Cc:
phosek@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature



Sign in to add a comment

Function selection without platform should always return first impl

Project Member Reported by qyears...@chromium.org, Mar 22 2018 
Right now for Spacey we have one Impl which is supposed to be for UBUNTU, although it's actually kind of platform-independent.

Changes I'd like to make:
 1. make it so that we can specify ANY or omit platform in the selections part of the config. For Spacey, this should return the first and only impl.
 2. Change all project configs with Spacey to omit platform (or specify it as ANY).
 3. Change the First impl of Spacey et al so that they provide for "ANY", since all analyzers so far are platform-independent.
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 24 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/62989db5d8c75035dac710cd34008183a7ebc2b6

commit 62989db5d8c75035dac710cd34008183a7ebc2b6
Author: Quinten Yearsley <qyearsley@chromium.org>
Date: Tue Apr 24 20:22:47 2018

Change analyzer lookup to return first implementation by default

Reason: In project configs, we currently have a lot of:

  selections {
    function: ...
    platform: UBUNTU
  }

where we don't actually care about about the platform, since all
the analyzers so far are platform-independent.

If no platform is specified, then the platform field is 0, which
corresponds to Platform_ANY. If we're trying to get an implementation
for ANY platform, we could just return the first.

Bug: 824948
Change-Id: Ib4c040540198d90a5ee6a1bbf20f90d420dbfd0b
Reviewed-on: https://chromium-review.googlesource.com/1013092
Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org>
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>

[modify] https://crrev.com/62989db5d8c75035dac710cd34008183a7ebc2b6/go/src/infra/tricium/api/v1/config_helpers.go
[modify] https://crrev.com/62989db5d8c75035dac710cd34008183a7ebc2b6/go/src/infra/tricium/api/v1/config_helpers_test.go

Comment 2 by qyears...@chromium.org, Jun 15 2018

Components: Infra>Platform>Tricium

Comment 3 by qyears...@chromium.org, Jun 15 2018

Components: -Infra>CodeAnalysis
Labels: -Tricium
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 3 

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/272b0abdb3db62843e2e5cdfa82e0eba9127d5b6

commit 272b0abdb3db62843e2e5cdfa82e0eba9127d5b6
Author: Quinten Yearsley <qyearsley@chromium.org>
Date: Tue Jul 03 16:52:06 2018

[tricium] Consider ANY platform to be supported by all functions

This is related to a past change which was never properly tested:
https://chromium-review.googlesource.com/c/infra/infra/+/1013092

This would allow us to not specify the platform of the function
in project configs when we don't care about the platform.

Bug: 824948
Change-Id: Ic8b4b4e5875aa5444dd9e8a7ee1a629e9adc1566
Reviewed-on: https://chromium-review.googlesource.com/1123111
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
Commit-Queue: Quinten Yearsley <qyearsley@chromium.org>

[modify] https://crrev.com/272b0abdb3db62843e2e5cdfa82e0eba9127d5b6/go/src/infra/tricium/api/v1/config_helpers.go
[modify] https://crrev.com/272b0abdb3db62843e2e5cdfa82e0eba9127d5b6/go/src/infra/tricium/api/v1/config_helpers_test.go
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 3

Labels: merge-merged-config
The following revision refers to this bug:
  https://chromium.googlesource.com/playground/gerrit-tricium/+/0a3c6bcd3a9e728c057f4f6f50d3c72fc691a386

commit 0a3c6bcd3a9e728c057f4f6f50d3c72fc691a386
Author: Quinten Yearsley <qyearsley@chromium.org>
Date: Tue Jul 03 17:02:43 2018

[tricium] Try omitting the platform in selections in project config

Bug: 824948
Change-Id: I09f75370cebf7f9c47b1a18cc02783a9cc1097a9
[modify] https://crrev.com/0a3c6bcd3a9e728c057f4f6f50d3c72fc691a386/tricium-dev.cfg

Comment 6 by qyears...@chromium.org, Jul 3

Owner: qyears...@chromium.org
Status: Assigned (was: Available)
After that, I got a panic:

Caught panic during handling of "/launcher/internal/launch": runtime error: invalid memory address or nil pointer dereference at infra/tricium/appengine/common/config/generate.go:187 +0x1e2.

Specifically, this is when dereferencing the platform returned by tricium.LookupPlatform(sc, s.Platform).

So, omitting the platform in selections in the project config still doesn't work.

Comment 7 by qyears...@chromium.org, Aug 25

Owner: ----
Status: Available (was: Assigned)
►

Sign in to add a comment