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

Add thrift 0.13.0 #8307

Merged
merged 6 commits into from
Sep 1, 2021
Merged

Add thrift 0.13.0 #8307

merged 6 commits into from
Sep 1, 2021

Conversation

msosyak
Copy link
Contributor

@msosyak msosyak commented Aug 2, 2021

Why I did it

To bump thrift version to 0.13.0, to fix some dependencies issues.

How I did it

As there are dependencies between thrift and saithrinft server (https://github.com/opencomputeproject/SAI/tree/bf3630316cbef484e1015478b5746067bac4e2b8/test/saithrift) which is used by syncd-rpc to update thrift version, I also need to make changes in saithrinft server, and then SAI ref point should be updated in sairedis, and then sairedis ref point should be updated too. It is too many change, so I decided to add thrift 0.13.0 as separeate target to be able to work and test father changes in saithrinft and one when appropriate changes will be merged to SAI and ref points will be updated I will squash this and the old thrift target. I was not able to build thrift deb pkg by original rules, so I copied debian folder from the old version and tune it for newer one.

How to verify it

make init
make configure PLATFORM=vs
make target/debs/buster/libthrift_0.13.0_amd64.deb

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106

Description for the changelog

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

 /\__/\
(=^.^=)
 (")(")_/

@msosyak
Copy link
Contributor Author

msosyak commented Aug 9, 2021

@qiluo-msft @xumia Please review

@qiluo-msft
Copy link
Collaborator

qiluo-msft commented Aug 9, 2021

Do you get the files from upstream? If mostly yes, do not copy them, instead, download it from a upstream repo, and apply your patch, and then build.

ref: https://github.com/Azure/sonic-buildimage/tree/master/src/initramfs-tools


In reply to: 895057166

@msosyak
Copy link
Contributor Author

msosyak commented Aug 9, 2021

I was not able to build thrift deb pkg by original rules https://github.com/apache/thrift/tree/0.13.0/debian , so I copied debian folder from the old version https://github.com/Azure/sonic-buildimage/blob/91432fff65347464d5e67cb80a39493acb91e570/src/thrift/Makefile#L19 and tune it for a newer one. I have tried to add patches to not copy but download and patch. But there are many changes including files renaming and deleting, so the copy of all debian folders looks more clear.

I have also tried to search for the newer version of thrift packages on https://sonicstorage.blob.core.windows.net/packages/debian, but looks like I do not have permission to browse the storage or do not found the proper link to do it. Will be very appreciate if you share the info on how to browse https://sonicstorage.blob.core.windows.net/packages/debian and push/update files there if it is possible.

@qiluo-msft
Copy link
Collaborator

qiluo-msft commented Aug 10, 2021

Did you try https://packages.debian.org/bullseye/libthrift-0.13.0?
Download Source Package thrift:
[thrift_0.13.0-6.dsc]
[thrift_0.13.0.orig.tar.gz]
[thrift_0.13.0-6.debian.tar.xz]


In reply to: 895776613

@msosyak msosyak force-pushed the myron/thrift_13 branch 2 times, most recently from 0e2450d to 0e74ee1 Compare August 10, 2021 10:41
@msosyak
Copy link
Contributor Author

msosyak commented Aug 10, 2021

No, I haven't tried. I missed that there is an archive with the source code. Just tried and it works. Thank you for the tip. I have updated PR to use source archive from debian.org

rules/thrift_0_13_0.dep Outdated Show resolved Hide resolved
@msosyak msosyak requested a review from qiluo-msft August 13, 2021 07:46
qiluo-msft
qiluo-msft previously approved these changes Aug 13, 2021
* use wget -O instead of wildcard remove
* remove lines instead of comment in patch
@msosyak
Copy link
Contributor Author

msosyak commented Aug 18, 2021

@qiluo-msft Your approval was dismissed when I force pushed the latest commit to retrigger the CI as it was failed.
Not the CI passed, so could we merge this?

@msosyak
Copy link
Contributor Author

msosyak commented Aug 31, 2021

@qiluo-msft Do you have some concerns or could we merge this?

@qiluo-msft qiluo-msft merged commit 0ab28bf into sonic-net:master Sep 1, 2021
RyoYang pushed a commit to RyoYang/sonic-buildimage that referenced this pull request Dec 24, 2021
#### Why I did it
To bump thrift version to 0.13.0, to fix some dependencies issues.

#### How I did it
As there are dependencies between thrift and saithrinft server  (https://github.com/opencomputeproject/SAI/tree/bf3630316cbef484e1015478b5746067bac4e2b8/test/saithrift) which is used by syncd-rpc to update thrift version, I also need to make changes in saithrinft server, and then SAI ref point should be updated in sairedis, and then sairedis ref point should be updated too. It is too many change, so I decided to add thrift 0.13.0 as separeate target to be able to work and test father changes in saithrinft and one when appropriate changes will be merged to SAI and ref points will be updated I will squash this and the old thrift target.  I was not able to build thrift deb pkg by original rules, so I copied `debian `folder from the old version and tune it for newer one.

#### How to verify it
```
make init
make configure PLATFORM=vs
make target/debs/buster/libthrift_0.13.0_amd64.deb
```

```
RyoYang pushed a commit to RyoYang/sonic-buildimage that referenced this pull request Dec 24, 2021
#### Why I did it
To bump thrift version to 0.13.0, to fix some dependencies issues.

#### How I did it
As there are dependencies between thrift and saithrinft server  (https://github.com/opencomputeproject/SAI/tree/bf3630316cbef484e1015478b5746067bac4e2b8/test/saithrift) which is used by syncd-rpc to update thrift version, I also need to make changes in saithrinft server, and then SAI ref point should be updated in sairedis, and then sairedis ref point should be updated too. It is too many change, so I decided to add thrift 0.13.0 as separeate target to be able to work and test father changes in saithrinft and one when appropriate changes will be merged to SAI and ref points will be updated I will squash this and the old thrift target.  I was not able to build thrift deb pkg by original rules, so I copied `debian `folder from the old version and tune it for newer one.

#### How to verify it
```
make init
make configure PLATFORM=vs
make target/debs/buster/libthrift_0.13.0_amd64.deb
```

```
RyoYang pushed a commit to RyoYang/sonic-buildimage that referenced this pull request Dec 24, 2021
#### Why I did it
To bump thrift version to 0.13.0, to fix some dependencies issues.

#### How I did it
As there are dependencies between thrift and saithrinft server  (https://github.com/opencomputeproject/SAI/tree/bf3630316cbef484e1015478b5746067bac4e2b8/test/saithrift) which is used by syncd-rpc to update thrift version, I also need to make changes in saithrinft server, and then SAI ref point should be updated in sairedis, and then sairedis ref point should be updated too. It is too many change, so I decided to add thrift 0.13.0 as separeate target to be able to work and test father changes in saithrinft and one when appropriate changes will be merged to SAI and ref points will be updated I will squash this and the old thrift target.  I was not able to build thrift deb pkg by original rules, so I copied `debian `folder from the old version and tune it for newer one.

#### How to verify it
```
make init
make configure PLATFORM=vs
make target/debs/buster/libthrift_0.13.0_amd64.deb
```

```
richardyu-ms pushed a commit to richardyu-ms/sonic-buildimage that referenced this pull request Jan 5, 2022
#### Why I did it
To bump thrift version to 0.13.0, to fix some dependencies issues.

#### How I did it
As there are dependencies between thrift and saithrinft server  (https://github.com/opencomputeproject/SAI/tree/bf3630316cbef484e1015478b5746067bac4e2b8/test/saithrift) which is used by syncd-rpc to update thrift version, I also need to make changes in saithrinft server, and then SAI ref point should be updated in sairedis, and then sairedis ref point should be updated too. It is too many change, so I decided to add thrift 0.13.0 as separeate target to be able to work and test father changes in saithrinft and one when appropriate changes will be merged to SAI and ref points will be updated I will squash this and the old thrift target.  I was not able to build thrift deb pkg by original rules, so I copied `debian `folder from the old version and tune it for newer one.

#### How to verify it
```
make init
make configure PLATFORM=vs
make target/debs/buster/libthrift_0.13.0_amd64.deb
```

```
@richardyu-ms richardyu-ms mentioned this pull request Jan 5, 2022
5 tasks
richardyu-ms added a commit that referenced this pull request Jan 6, 2022
* Add thrift 0.13.0  (#8307)

#### Why I did it
To bump thrift version to 0.13.0, to fix some dependencies issues.

#### How I did it
As there are dependencies between thrift and saithrinft server  (https://github.com/opencomputeproject/SAI/tree/bf3630316cbef484e1015478b5746067bac4e2b8/test/saithrift) which is used by syncd-rpc to update thrift version, I also need to make changes in saithrinft server, and then SAI ref point should be updated in sairedis, and then sairedis ref point should be updated too. It is too many change, so I decided to add thrift 0.13.0 as separeate target to be able to work and test father changes in saithrinft and one when appropriate changes will be merged to SAI and ref points will be updated I will squash this and the old thrift target.  I was not able to build thrift deb pkg by original rules, so I copied `debian `folder from the old version and tune it for newer one.

#### How to verify it
```
make init
make configure PLATFORM=vs
make target/debs/buster/libthrift_0.13.0_amd64.deb
```

```

* Correct the pkg name for thrift.0.13.0

Correct thrift.0.13.0 dependent package name.
In previous code, the buildout target was named as PYTHON3_THRIFT_0_13_0
But when add the prackage to LIBTHRIFT_0_13_0, it typo as PYTHON_THRIFT_0_13_0

Co-authored-by: Myron Sosyak <[email protected]>
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.

2 participants