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

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

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?

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

@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.

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>
@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

@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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants