Opened 10 years ago
Closed 9 years ago
#47231 closed enhancement (fixed)
resolve minor nits with par
Reported by: | grimreaper (Eitan Adler) | Owned by: | qbarnes (Quentin Barnes) |
---|---|---|---|
Priority: | Low | Milestone: | |
Component: | ports | Version: | 2.3.3 |
Keywords: | haspatch | Cc: | Ionic (Mihai Moldovan) |
Port: | par |
Description
Attachments (2)
Change History (14)
Changed 10 years ago by grimreaper (Eitan Adler)
Attachment: | qbarnes.diff added |
---|
comment:1 Changed 10 years ago by Ionic (Mihai Moldovan)
Cc: | qbarnes@… removed |
---|---|
Owner: | changed from macports-tickets@… to qbarnes@… |
Priority: | Normal → Low |
comment:2 Changed 10 years ago by qbarnes (Quentin Barnes)
comment:3 Changed 10 years ago by grimreaper (Eitan Adler)
this was part of a project to add consistency. I was told the 'correct' way was 0755 for my own ports and then just fixed it for all the others too.
comment:4 Changed 10 years ago by Ionic (Mihai Moldovan)
The default UNIX convention for binaries installed in bindir
is 0755
.
comment:5 Changed 10 years ago by larryv (Lawrence Velázquez)
This is correct and can be expressed by simply deleting “-m 555” (xinstall defaults to 0755), but it doesn’t merit a revbump.
Changed 10 years ago by grimreaper (Eitan Adler)
Attachment: | qbarnes.diff added |
---|
comment:6 Changed 10 years ago by grimreaper (Eitan Adler)
updated diff to not have revbump and remove explicit -m 555
comment:7 follow-up: 8 Changed 10 years ago by qbarnes (Quentin Barnes)
Ah, ok. Nothing technically wrong with the way it was other than violating convention. It is a change to bring it into alignment with other packages. In that case, sounds good then, especially with removing the -m option letting xinstall default rather than rehardcoding a new value.
So not bumping the rev is simply to save triggering a rebuild for people for a inconsequential update?
If so, the patch looks good to me and should be applied. What's the next step? I don't have commit access.
comment:8 Changed 10 years ago by Ionic (Mihai Moldovan)
Replying to qbarnes@…:
Ah, ok. Nothing technically wrong with the way it was other than violating convention.
Yep, it's merely an enhancement.
It is a change to bring it into alignment with other packages.
The vast majority of packages either don't specify the mode or use 755
. There are some ports currently installing with 555
for binaries, but that's what these tickets are for (or somesuch.)
In that case, sounds good then, especially with removing the -m option letting xinstall default rather than rehardcoding a new value.
So not bumping the rev is simply to save triggering a rebuild for people for a inconsequential update?
Well, it's for saving triggering a rebuild for people/users... mind you, that's "wrong" to my mind, as the filesystem and Portfile
will end up diverging, even though this diversion is rather cosmetic and not influencing how a package works. I, personally, still prefer consistency over the comfort of not having to rebuild packages. Most other MacPorts developers seem to think otherwise, however, so I don't really care.
If so, the patch looks good to me and should be applied. What's the next step? I don't have commit access.
I can apply everything, as long as the maintainer gives explicit permission.
comment:9 Changed 9 years ago by casr+macports@…
Replying to ionic@…:
Replying to qbarnes@…:
If so, the patch looks good to me and should be applied. What's the next step? I don't have commit access.
I can apply everything, as long as the maintainer gives explicit permission.
qbarnes@ is the maintainer so I think that looks like permission to apply the patch :)
comment:10 Changed 9 years ago by qbarnes (Quentin Barnes)
Is there a reason this patch still hasn't be integrated?
Does something else I need to do to mark it as ready to get it in the queue?
comment:11 Changed 9 years ago by Ionic (Mihai Moldovan)
Yeah... actually, I waited for explicit permission, as said before. I'll take your latest comment as that and will apply the patch (in two steps.)
comment:12 Changed 9 years ago by Ionic (Mihai Moldovan)
Resolution: | → fixed |
---|---|
Status: | new → closed |
Why is it a good idea to change the perms on the executable from 0555 to 0755?