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

Migrate sub directory to fluent-package #448

Merged
merged 3 commits into from
Apr 27, 2023

Conversation

kenhys
Copy link
Contributor

@kenhys kenhys commented Apr 25, 2023

NOTE:

  • package name (td-agent) is not changed yet
    * PACKAGE and ALL_PACKAGE use fluent-package in Rakefile,
    but it is just used to change sub directory.
  • internal path of /opt/td-agent is not changed yet

So it keeps compatibility.

@ashie
Copy link
Member

ashie commented Apr 25, 2023

I expect to pass CI in this PR.

Could you rename also directory names in Rakefile and .github/workflows/?

PACKAGES = [
"td-agent",
]
APT_SOURCE_PACKAGES = [
"fluentd-apt-source"
]
ALL_PACKAGE = [
"td-agent",
"fluentd-apt-source",
]

path: td-agent/apt/repositories

Probably they aren't affect to built package names, so that tests will pass by only fixing them.
(AFAIK the code under td-agent directory isn't aware sub directory name.)

@kenhys kenhys marked this pull request as draft April 25, 2023 05:24
fluent-package/Rakefile Outdated Show resolved Hide resolved
@kenhys kenhys force-pushed the rename-to-fluent-package branch from f821e2d to 349f97b Compare April 25, 2023 08:43
@kenhys
Copy link
Contributor Author

kenhys commented Apr 25, 2023

TODO: fix test case

@kenhys kenhys force-pushed the rename-to-fluent-package branch 8 times, most recently from a972071 to 4bc9909 Compare April 26, 2023 07:03
@cosmo0920
Copy link
Contributor

cosmo0920 commented Apr 26, 2023

One thing is existing that I've been wondering for.
Should we add a conflicts or replacing definition for rpm/deb package rules?
If we could handle this idea, we should provide such upgrading option to proceed seemless upgrade from old version of td-agent. Or, the future versions provide just breaking changes including package name changing and bumping up of major version?

@kenhys
Copy link
Contributor Author

kenhys commented Apr 26, 2023

Should we add a conflicts or replacing definition for rpm/deb package rules?

Yes, it should be done. but all of such changes in a single PR are not appropriate (it makes it hard to review, and so on), so they will be solved in another PR.

This PR focus on a small task on purpose.

@kenhys kenhys marked this pull request as ready for review April 26, 2023 09:20
@kenhys
Copy link
Contributor Author

kenhys commented Apr 26, 2023

Even though many timeouts occurs, now CI has passed.

Copy link
Member

@ashie ashie left a comment

Choose a reason for hiding this comment

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

I prefer getting source directory by __dir__ instead of defining PACKAGE_DIR.

fluent-package/Rakefile Outdated Show resolved Hide resolved
fluent-package/Rakefile Outdated Show resolved Hide resolved
fluent-package/Rakefile Outdated Show resolved Hide resolved
fluent-package/Rakefile Outdated Show resolved Hide resolved
fluent-package/config.rb Outdated Show resolved Hide resolved
@kenhys
Copy link
Contributor Author

kenhys commented Apr 27, 2023

I prefer getting source directory by dir instead of defining PACKAGE_DIR.

okay, I'll try to fix it. Thanks.

@kenhys kenhys force-pushed the rename-to-fluent-package branch from 3807650 to 58a8f04 Compare April 27, 2023 04:55
@kenhys
Copy link
Contributor Author

kenhys commented Apr 27, 2023

With checking local build, there is no difference like this:
(diff with current master and PR 448)

+ rm -f tdagent5deb.orig tdagent5rpm.orig tdagent5deb.rev tdagent5rpm.rev
+ debdiff
+ SRC=fluent-package-builder.master/td-agent/apt/repositories/debian/pool/bullseye/main/t/td-agent/td-agent_5.0.0-1_amd64.deb
+ DEST=fluent-package-builder.work/fluent-package/apt/repositories/debian/pool/bullseye/main/t/td-agent/td-agent_5.0.0-1_amd64.deb
+ dpkg-deb -c fluent-package-builder.master/td-agent/apt/repositories/debian/pool/bullseye/main/t/td-agent/td-agent_5.0.0-1_amd64.deb
+ gawk '{print $6}'
+ sort
+ dpkg-deb -c fluent-package-builder.work/fluent-package/apt/repositories/debian/pool/bullseye/main/t/td-agent/td-agent_5.0.0-1_amd64.deb
+ gawk '{print $6}'
+ sort
+ diff -u tdagent5deb.orig tdagent5deb.rev
+ rpmdiff
+ SRC=fluent-package-builder.master/td-agent/yum/repositories/centos/7/x86_64/Packages/td-agent-5.0.0-1.el7.x86_64.rpm
+ DEST=fluent-package-builder.work/fluent-package/yum/repositories/centos/7/x86_64/Packages/td-agent-5.0.0-1.el7.x86_64.rpm
+ rpm -qpl fluent-package-builder.master/td-agent/yum/repositories/centos/7/x86_64/Packages/td-agent-5.0.0-1.el7.x86_64.rpm
+ grep -v build-id
+ sort
+ rpm -qpl fluent-package-builder.work/fluent-package/yum/repositories/centos/7/x86_64/Packages/td-agent-5.0.0-1.el7.x86_64.rpm
+ grep -v build-id
+ sort
+ diff -u tdagent5rpm.orig tdagent5rpm.rev

@kenhys
Copy link
Contributor Author

kenhys commented Apr 27, 2023

All checks have passed again.

Copy link
Member

@ashie ashie left a comment

Choose a reason for hiding this comment

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

LGTM

@ashie
Copy link
Member

ashie commented Apr 27, 2023

@kenhys Could you revise the first comment to reflect the current status? (Since we migrated merge style to Squash and merge, the first comment will be used as the commit message.)

kenhys added 3 commits April 27, 2023 15:16
NOTE:

* package name (td-agent) is not changed yet
  * PACKAGE and ALL_PACKAGE use fluent-package in Rakefile,
  but it is just used to change sub directory.
* internal path of /opt/td-agent is not changed yet

So it keeps compatibility.

Signed-off-by: Kentaro Hayashi <[email protected]>
Signed-off-by: Kentaro Hayashi <[email protected]>
Signed-off-by: Kentaro Hayashi <[email protected]>
@kenhys kenhys force-pushed the rename-to-fluent-package branch from 58a8f04 to 243fe8c Compare April 27, 2023 06:18
@kenhys
Copy link
Contributor Author

kenhys commented Apr 27, 2023

@ashie

Update the commit message, is it reasonable?

@ashie
Copy link
Member

ashie commented Apr 27, 2023

Please revise also the title & first comment of this PR.

@kenhys kenhys changed the title Rename to fluent-package Migrate sub directory to fluent-package Apr 27, 2023
@kenhys
Copy link
Contributor Author

kenhys commented Apr 27, 2023

Fixed now.

@ashie ashie merged commit b0a2e99 into fluent:master Apr 27, 2023
@ashie
Copy link
Member

ashie commented Apr 27, 2023

Thanks!

1 similar comment
@kenhys
Copy link
Contributor Author

kenhys commented Apr 27, 2023

Thanks!

kenhys added a commit to kenhys/fluent-package-builder that referenced this pull request Apr 27, 2023
This PR is follow-up of fluent#448 .

NOTE:
* The internal path (such as /opt/td-agent) is not changed.

It keeps compatibility.

Signed-off-by: Kentaro Hayashi <[email protected]>
kenhys added a commit to kenhys/fluent-package-builder that referenced this pull request Apr 28, 2023
This PR is follow-up of fluent#448 .

NOTE:
* The internal path (such as /opt/td-agent) is not changed.
* Install/Upgrade package test is now testing
  from td-agent 4 to fluent-package 5.

It keeps compatibility.

Signed-off-by: Kentaro Hayashi <[email protected]>
kenhys added a commit to kenhys/fluent-package-builder that referenced this pull request Apr 28, 2023
This PR is follow-up of fluent#448 .

NOTE:
* The internal path (such as /opt/td-agent) is not changed.
* Install/Upgrade package test is now testing
  from td-agent 4 to fluent-package 5.
* It provides transitional td-agent package intentionally..
  It should be removed after a while.

It keeps compatibility.

Signed-off-by: Kentaro Hayashi <[email protected]>
ashie added a commit that referenced this pull request May 2, 2023
The package name (td-agent) is changed to fluent-package.

This PR is follow-up of #448 .

NOTE:
* The internal path (such as /opt/td-agent) is not changed.
* Install/Upgrade package test is now testing
  from td-agent 4 to fluent-package 5.
* It provides transitional td-agent package intentionally..
  It should be removed after a while.

It keeps compatibility.

---------

Signed-off-by: Kentaro Hayashi <[email protected]>
Co-authored-by: Takuro Ashie <[email protected]>
@kenhys kenhys deleted the rename-to-fluent-package branch May 2, 2023 06:30
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.

4 participants