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

[build]: put stretch debian packages under target/debs/stretch/ #2519

Merged
merged 4 commits into from
Feb 5, 2019

Conversation

lguohan
Copy link
Collaborator

@lguohan lguohan commented Feb 4, 2019

debian packages for stretch are separated from jessie debian packages

Signed-off-by: Guohan Lu gulv@microsoft.com

- What I did

- How I did it

  • in stretch build phase, all debian packages built in that stage are placed under target/debs/stretch directory.
  • for python-based debian packages, since they are really the same for jessie and stretch, they are placed under target/python-debs directory.

- How to verify it
build the image and test

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@lguohan lguohan requested a review from jleveque February 4, 2019 05:27
debian packages for stretch are separated from jessie debian packages

Signed-off-by: Guohan Lu <gulv@microsoft.com>
Signed-off-by: Guohan Lu <gulv@microsoft.com>
Signed-off-by: Guohan Lu <gulv@microsoft.com>
@jleveque
Copy link
Contributor

jleveque commented Feb 4, 2019

@lguohan: Could you please provide a brief description of the new intended package directory structure and build process so that I can better review the changes?

@NStetskovych-zz
Copy link
Contributor

what about other platforms? despite mlnx and vs?

Signed-off-by: Guohan Lu <gulv@microsoft.com>
@lguohan
Copy link
Collaborator Author

lguohan commented Feb 4, 2019

@NStetskovych, it looks cavium platform did not put the kernel drivers into stretch directory. I fixed the problem, however, I do not really have a way to validate the image. can you check?

@lguohan
Copy link
Collaborator Author

lguohan commented Feb 4, 2019

@jleveque , add description of the changes.

@lguohan lguohan merged commit f206650 into sonic-net:master Feb 5, 2019
@nathanalderson
Copy link

I believe this PR introduced a new build failure on top of a (long-existing) build failure in the p4 platform. See https://sonic-jenkins.westus2.cloudapp.azure.com/job/p4/job/buildimage-p4-all/950/.

@@ -103,6 +103,7 @@ DOCKER_BUILD = docker build --no-cache \

SONIC_BUILD_INSTRUCTION := make \
-f slave.mk \
BLDENV=$(BLDENV) \
Copy link
Collaborator

Choose a reason for hiding this comment

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

BLDENV [](start = 27, length = 6)

slave.mk could detect the BLDENV from /etc/os-release. We should not treat it as a parameter.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good suggestion, I'll try later.

@@ -24,10 +24,18 @@ SRC_PATH = src
RULES_PATH = rules
TARGET_PATH = target
DOCKERS_PATH = dockers
ifdef BLDENV
Copy link
Collaborator

@qiluo-msft qiluo-msft Apr 16, 2019

Choose a reason for hiding this comment

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

BLDENV [](start = 6, length = 6)

We should assume BLDENV always defined. either STRETCH or JESSIE, and SID in future.

If keep backward compatibility, we could create target/debs as symlink to target/jessie/debs

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually backward compatibility should not be an issue. It is easy to fix build script or jenkins script. If this is true, we don't need a symlink.


In reply to: 275602303 [](ancestors = 275602303)

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.

5 participants