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

Jump to bottom

android-tools-adbd.service: Change /var to /etc in ConditionPathExists #862

Open
wants to merge 3 commits into
base: scarthgap
Choose a base branch
from
Open

android-tools-adbd.service: Change /var to /etc in ConditionPathExists #862

wants to merge 3 commits into from

Conversation

quic-raghuvar
Copy link
Contributor

If android-tools-adbd.service service needs to be up upon boot, then the path assigned to ConditionPathExists must be present at boot time. This means that the path set to ConditionPathExists must be created at build time itself. /etc is a better place to keep files and directories that are created at build time rather than /var. /var is expected to house files that are created at run time.

Hence, change ConditionPathExists=/var/usb-debugging-enabled to ConditionPathExists=/etc/usb-debugging-enabled

@lumag
Copy link
Contributor

lumag commented Aug 22, 2024

Missing DCO tag.

@lumag
Copy link
Contributor

lumag commented Aug 26, 2024

Please format the commit message wrapping the text somewhere between 72 and 77 chars. Please fix your editor setup.

@lumag
Copy link
Contributor

lumag commented Aug 30, 2024

@quic-raghuvar any updates? Or was it fire-and-forget request?

@quic-raghuvar
android-tools-adbd.service: Change /var to /etc in ConditionPathExists
37e3695 
If android-tools-adbd.service service needs to be up upon boot, then the path assigned to ConditionPathExists must be present at boot time. This means that the path set to ConditionPathExists must be created at build time itself. /etc is a better place to keep files and directories that are created at build time rather than /var. /var is expected to house files that are created at run time.

Hence, change ConditionPathExists=/var/usb-debugging-enabled to ConditionPathExists=/etc/usb-debugging-enabled

Signed-off-by: Raghuvarya S <quic_raghuvar@quicinc.com>
@quic-raghuvar
Copy link
Contributor Author

@lumag Updated Signed-off-by in the git commit description.

@quic-raghuvar
Copy link
Contributor Author

Please format the commit message wrapping the text somewhere between 72 and 77 chars. Please fix your editor setup.

Currently commit message is 70 characters long. Commit message is getting truncated when updated to something of 76 characters, which is between 72 and 77.

@lumag
Copy link
Contributor

lumag commented Sep 2, 2024

Please format the commit message wrapping the text somewhere between 72 and 77 chars. Please fix your editor setup.

Currently commit message is 70 characters long. Commit message is getting truncated when updated to something of 76 characters, which is between 72 and 77.

It is not, see https://github.com/openembedded/meta-openembedded/commit/37e3695eb5d56a846af123a26df86ea749a4dd52.patch

@lumag
Copy link
Contributor

lumag commented Sep 2, 2024 •
edited
Loading

@quic-raghuvar also the Author field is still incorrect, it contains account name instead of the full name.
BTW: is your real name component just a single letter 'S'?

@quic-raghuvar
Copy link
Contributor Author

Yes. Full name is "Raghuvarya S"

@lumag
Copy link
Contributor

lumag commented Sep 2, 2024

Please fix the Author of the commit.

@lumag lumag mentioned this pull request Sep 2, 2024
Open
@quic-raghuvar
android-tools-adbd.service: Change /var to /etc in ConditionPathExists
6855e12 
If android-tools-adbd.service service needs to be up upon boot, then the
path assigned to ConditionPathExists must be present at boot time. This
means that the path set to ConditionPathExists must be created at build
time itself. /etc is a better place to keep files and directories that are
created at build time rather than /var. /var is expected to house files
that are created at run time.

Hence, change ConditionPathExists=/var/usb-debugging-enabled to
ConditionPathExists=/etc/usb-debugging-enabled
Hence, change ConditionPathExists=/var/usb-debugging-enabled to ConditionPathExists=/etc/usb-debugging-enabled

Signed-off-by: Raghuvarya S <quic_raghuvar@quicinc.com>
@quic-raghuvar
Merge branch 'patch-3' of https://github.com/quic-raghuvar/meta-opene…
4c6fe01 
…mbedded into pr/862
@lumag
Copy link
Contributor

lumag commented Sep 2, 2024

@quic-raghuvar how did you end up with the merge commit again? Anyway, the author data still shows your account name rather than your full name.

@quic-raghuvar
Copy link
Contributor Author

I am not sure how it ended up with a merge. Also, I have updated the author field in the github configuration as part of GH Desktop. Not sure why it isn't getting reflected here.

@lumag
Copy link
Contributor

lumag commented Sep 2, 2024

Please use command line client if you are unsure in what is going on.

@quic-raghuvar
Copy link
Contributor Author

Okay

@lumag
Copy link
Contributor

lumag commented Sep 2, 2024

For the author part, see the 'From' here: https://github.com/openembedded/meta-openembedded/commit/6855e129ae9891b3a8dd86725083d1a08210a7c2.patch

@lumag
Copy link
Contributor

lumag commented Sep 5, 2024

@quic-raghuvar I hate to say that, but please, in future, when making contributions, please take a broader look on what you are doing.
This is on a fresh checkout of meta-openembedded, master branch. You have changed the names that are being tested, but didn't change the names that are being created. This is not really a quality and the attitude that we expect when somebody contributes upstream.

$ git grep usb-debugging-ena
meta-oe/dynamic-layers/selinux/recipes-devtool/android-tools/android-tools/android-tools-adbd.service:ConditionPathExists=/etc/usb-debugging-enabled
meta-oe/dynamic-layers/selinux/recipes-devtool/android-tools/android-tools_29.0.6.r14.bb:    touch ${IMAGE_ROOTFS}/var/usb-debugging-enabled
meta-oe/recipes-devtools/android-tools/android-tools/android-tools-adbd.service:ConditionPathExists=/etc/usb-debugging-enabled
meta-oe/recipes-devtools/android-tools/android-tools_5.1.1.r37.bb:    touch ${IMAGE_ROOTFS}/var/usb-debugging-enabled

I'll send a patch fixing this. And I'm leaving all the scarthgap pull requests up to maintainers discretion. Ideally all those commits either should be squashed or should go as a single batch, not as a series of several pull requests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@akuster akuster

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@quic-raghuvar @lumag @akuster