-
Notifications
You must be signed in to change notification settings - Fork 402
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
iox-#625 set file lock directory to /tmp/ #626
iox-#625 set file lock directory to /tmp/ #626
Conversation
Signed-off-by: Christian Eltzschig <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #626 +/- ##
==========================================
- Coverage 72.89% 72.86% -0.03%
==========================================
Files 312 312
Lines 10819 10819
Branches 1941 1941
==========================================
- Hits 7886 7883 -3
- Misses 2170 2172 +2
- Partials 763 764 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just to unblock developers who are not using ubuntu but we need to check the impact of this on QNX
@@ -45,7 +45,7 @@ enum class FileLockError | |||
INTERNAL_LOGIC_ERROR, | |||
}; | |||
|
|||
constexpr char PATH_PREFIX[] = "/var/lock/"; | |||
constexpr char PATH_PREFIX[] = "/tmp/"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the file system hierarchy standard defined by the Linux foundation, lock files shall be placed under /var/lock
. Seems like your distribution decided to choose another location. Mac OS prefers /var/tmp/
, this will be available with #598
QNX advises to use /var/lock
, but in our experience so far, it depends to the image, that is created.
Could you please add this with an #ifdef
for common platforms? installation.md#QNX
currently states that /var/locks
is used, in case you change it please update it there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mossmaurice sorry I read your comment after I merged the pull request :( ... but I will handle this comment nevertheless.
According to the file system hierarchy standard defined by the Linux foundation, lock files shall be placed under /var/locks
This is correct but the access rights of /var/lock
is in Gentoo, ArchLinux and OpenSuSe like the following:
drwxrwxr-x 5 root root 120 Mar 22 15:26 lock
# /var
This means the user which is starting RouDi has to be at least in the root group. On the other hand /tmp/
is for all users always read and writable and since the applications and roudi has started as an arbitrary user it is the perfect spot for the lock file in QNX, Mac OS and Linux. If you insist in using /var/lock
for the lock file we have to do the following things.
- Write
#ifdef
for the platforms, instead of using/tmp
which works for every platform (except Windows of course). - Add a chapter in the
installation.md
on howto add the user which is executing roudi to the lock management group of Linux, Mac OS (this will be a pain) and QNX.
If you do not object I would let the directory be /tmp
and would add this to the installation.md
since the file system hierarchy standard states the directory structure of a linux system and not where an application has to store the files. I admit it would be best to use the folders accordingly but it seems that on some linux systems we do not have access to /var/lock
but we will always have access to /tmp
.
By the way this also goes for QNX. The QNX boards of the past where also exactly in this way configured that everything was read only and we only had write access to /tmp
.
…lock-directory iox-eclipse-iceoryx#625 set file lock directory to /tmp/
Pre-Review Checklist for the PR Author
iox-#123-this-is-a-branch
)iox-#123 commit text
)git commit -s
)task-list-completed
)Notes for Reviewer
Checklist for the PR Reviewer
Post-review Checklist for the PR Author
References