Bug 192347 - [maintainer update] multimedia/universal-media-server: Update to 4.0.1 + FIXES
Summary: [maintainer update] multimedia/universal-media-server: Update to 4.0.1 + FIXES
Status: Closed Overcome By Events
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Bartek Rutkowski
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-08-02 21:07 UTC by dreamcat4
Modified: 2015-02-18 15:35 UTC (History)
2 users (show)

See Also:


Attachments
svn diff on head (2.60 KB, patch)
2014-08-02 21:07 UTC, dreamcat4
dreamcat4: maintainer-approval+
Details | Diff
universal-media-server.diff (3.56 KB, patch)
2014-08-05 12:59 UTC, Bartek Rutkowski
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description dreamcat4 2014-08-02 21:07:25 UTC
Created attachment 145274 [details]
svn diff on head

svn diff on head.
This patch is Maintainer Approved.

Fixing:

* rc.d script
* plist dies 'ums' user ownership
* Update to 4.0.1

Don't muck it up please.
Comment 1 John Marino freebsd_committer freebsd_triage 2014-08-03 11:29:42 UTC
Are there any test logs available?  preferably with stage-qa tests?

Am I seeing the pkg-plist diff correctly in that there are blank lines in the pkg-plist now?  I don't think those are going to fly
Comment 2 dreamcat4 2014-08-03 11:43:40 UTC
(In reply to John Marino from comment #1)
> Are there any test logs available?  preferably with stage-qa tests?

Yes:

https://redports.org/buildarchive/20140802204647-04291/

> Am I seeing the pkg-plist diff correctly in that there are blank lines in
> the pkg-plist now?  I don't think those are going to fly

The blank lines in pky-plist are only for readability the manual section at end of file. They can be taken out if you want. But it's nothing new. The original version of the port had them (4.0.0-b1) and was approved on that basis.

This patch puts back breakages in previous commit. There is no big cause to worry, this ways have been thoroughly tested on 4.0.0-b1, which was just like this patch ^^.

I'm honestly tired of other people trying to mess this up and 'so called fix things' that were not broken to begin with. Because delaying this commit for other small petty reasons like that is certainly going to lead to problem for other users. That is because of breakages committed just previously (without my consultation or approval).

If you want other type of 'stage-qa test' apart from reports build. Then unfortunately I don't know what you might mean, so please explain what that means. I build on reports.org build farm and manually on 9.2-RELEASE (inside a jail). And all is fine.


Please either:

a) take commit my, or else 

b) give it over to pi@FreeBSD.org (Kurt Jaeger?). Since  he knows this port the most well (and has committed for it before).
Comment 3 John Marino freebsd_committer freebsd_triage 2014-08-03 12:02:14 UTC
(In reply to dreamcat4 from comment #2)
> (In reply to John Marino from comment #1)
> > Are there any test logs available?  preferably with stage-qa tests?
> 
> Yes:
> 
> https://redports.org/buildarchive/20140802204647-04291/
>
> If you want other type of 'stage-qa test' apart from reports build. Then
> unfortunately I don't know what you might mean, so please explain what that
> means. I build on reports.org build farm and manually on 9.2-RELEASE (inside
> a jail). And all is fine.

Redports is good, but not the best.  Redports does not provide stage-qa checks yet, only poudriere does.  This means a port can potentially pass Redports but fail on the poudriere-based Jenkins so a committer isn't fully confident in a redports-only test.

But this one is risky because it's messing with /var which I don't think Redports tests well.


> > Am I seeing the pkg-plist diff correctly in that there are blank lines in
> > the pkg-plist now?  I don't think those are going to fly
> 
> The blank lines in pky-plist are only for readability the manual section at
> end of file. They can be taken out if you want. But it's nothing new. The
> original version of the port had them (4.0.0-b1) and was approved on that
> basis.
> 
> This patch puts back breakages in previous commit. There is no big cause to
> worry, this ways have been thoroughly tested on 4.0.0-b1, which was just
> like this patch ^^.
> 
> I'm honestly tired of other people trying to mess this up and 'so called fix
> things' that were not broken to begin with. Because delaying this commit for
> other small petty reasons like that is certainly going to lead to problem
> for other users. That is because of breakages committed just previously
> (without my consultation or approval).


What do you "muck this up"?
This port has 3 main commits, all with PRs coming from you.

If a committer changes what you submitted, it's because it didn't pass a test or wasn't done correctly.


Since there seems to be a lot more background to this than the PR suggests, I think you also need to illustrate what change that you didn't make that was added without your approval was committed and why it's wrong.

I'm going to recommend to whomever takes this PR that they run it though poudriere and not rely solely on redports.
Comment 4 dreamcat4 2014-08-03 12:38:21 UTC
(In reply to John Marino from comment #3)
> But this one is risky because it's messing with /var which I don't think
> Redports tests well.

If messing with var is risky, then what is the logic not to put it back to how it was previously, when was tested working fine??? Please explain.

BTW I am not setup to run poudriere. Mainly because it is too complex / too much hassle to run and install on my current system. I have very limited hardware available to me, which can only run on 9.2 (other reasons). If the FreeBSD project provides in future a public build service (or improve the current reports) with pourdiere on it, then of course I will be happy to make use of that.

> Since there seems to be a lot more background to this than the PR suggests,
> I think you also need to illustrate what change that you didn't make that
> was added without your approval was committed and why it's wrong.

Everything in this patch, except "java" category. Is not been tested by me. Is broken for example the PLIST_SUB references in the rc.d script. And does not improve anything anyways.

> I'm going to recommend to whomever takes this PR that they run it though
> poudriere and not rely solely on redports.

Sure. But at least, (if it is not too much trouble) then show me the poudriere build results / output by attaching it to this PR. Because reports is showing 0 issues. And my manual build (in it's own jail). Is showing 0 issues. And I have followed the manual testing steps  in Porters Handbook (the check-orphans, etc) to the letter.
Comment 5 John Marino freebsd_committer freebsd_triage 2014-08-03 13:18:30 UTC
(In reply to dreamcat4 from comment #4)
> (In reply to John Marino from comment #3)
> > But this one is risky because it's messing with /var which I don't think
> > Redports tests well.
> 
> If messing with var is risky, then what is the logic not to put it back to
> how it was previously, when was tested working fine??? Please explain.


This is getting to be too much drama for me.
You said you tested previous versions, presumably also with redports which I've already told you can't fully test everything.  The logic reason for changing a submission, which nobody does for fun, is that an error was found that redports didn't catch.  So obviously going back to that error is not an option.

This is conjecture, i didn't commit these.  I don't know for sure why anything was changed.

 
> BTW I am not setup to run poudriere. Mainly because it is too complex / too
> much hassle to run and install on my current system. I have very limited
> hardware available to me, which can only run on 9.2 (other reasons). 


It's actually not very complex.  It's not really a hassle either.  There's no problem in providing a poudriere report from 9.2 either.


> If the
> FreeBSD project provides in future a public build service (or improve the
> current reports) with pourdiere on it, then of course I will be happy to
> make use of that.


Which is in the plans.  I just don't know when.  And at some point it won't be optional, but rather a requirement for all submissions but there has to be good infrastructure in place to require that.




 
> > Since there seems to be a lot more background to this than the PR suggests,
> > I think you also need to illustrate what change that you didn't make that
> > was added without your approval was committed and why it's wrong.
> 
> Everything in this patch, except "java" category. Is not been tested by me.
> Is broken for example the PLIST_SUB references in the rc.d script. And does
> not improve anything anyways.

how is this change correct?
%%PORTDOCS%%@dirrmtry %%DOCSDIR%%

You'd better be able to remove that directory without question, so dirrmtry is wrong if it's not being shared with another port (which is rare)

DISTFILES=	UMS-${DISTVERSION}.tgz

That's equivalent.  You could have also defined USES+= tar:tgz though and gotten rid of the extract suffix


this stuff:
@exec mkdir -p %%UMS_PROFILE_PATH%%

is wrong.  it could be 8 levels deep since it's a variable.  You only attempt to remove one level.  But you've defined it unconditionally as /var/run/${PORTNAME} so what purpose does having it as a PLIST_SUB have?  I don't see one.


I haven't see the rc script, maybe all it needs is "var/run/universal-media-server" hardcoded to it.  That actually sounds like an oversight that needs fixing.  Most of the other stuff I don't see as mistakes.


> 
> > I'm going to recommend to whomever takes this PR that they run it though
> > poudriere and not rely solely on redports.
> 
> Sure. But at least, (if it is not too much trouble) then show me the
> poudriere build results / output by attaching it to this PR. Because reports
> is showing 0 issues. And my manual build (in it's own jail). Is showing 0
> issues. And I have followed the manual testing steps  in Porters Handbook
> (the check-orphans, etc) to the letter.

- Again, redports < poudriere.  You can pass in redports and fail poudriere.
- that implies you already ran make stage-qa which is good.  That's the check that's missing from redports
Comment 6 dreamcat4 2014-08-03 14:32:49 UTC
(In reply to John Marino from comment #5)
> how is this change correct?
> %%PORTDOCS%%@dirrmtry %%DOCSDIR%%
> 
> You'd better be able to remove that directory without question, so dirrmtry
> is wrong if it's not being shared with another port (which is rare)

I'm not sure I agree. The original entry is auto-generated. In other directories (which are not docs stuff), they don't remove without question. If the user has modified of put something of their own files there. So how is docs directory different? Also: that only matters IF the contents of the directory has been added to somehow. But the files put in there are static and we would not expect the directory to be non-empty. I just don't get it.

> DISTFILES=	UMS-${DISTVERSION}.tgz
> 
> That's equivalent.  You could have also defined USES+= tar:tgz though and
> gotten rid of the extract suffix

Yes it is. It also adds an extra line / variable to the port Makefile for no apparent reason whatsoever.

> this stuff:
> @exec mkdir -p %%UMS_PROFILE_PATH%%
> 
> is wrong.  it could be 8 levels deep since it's a variable.  You only
> attempt to remove one level.  But you've defined it unconditionally as

Ah OK then.

> /var/run/${PORTNAME} so what purpose does having it as a PLIST_SUB have?  I
> don't see one.

Used by rc.d script (like I mentioned just in my previous comment).

> I haven't see the rc script, maybe all it needs is
> "var/run/universal-media-server" hardcoded to it.  That actually sounds like
> an oversight that needs fixing.  Most of the other stuff I don't see as
> mistakes.

Yeah maybe. But there were no guideline for that in Porter's Handbook (that I could see)> And as the variable that is set in Makefile is currently set to be "/var/run/..." then functionally there is was no error at the time this change was introduced.

> - Again, redports < poudriere.  You can pass in redports and fail poudriere.
> - that implies you already ran make stage-qa which is good.  That's the
> check that's missing from redports

In the Porters handbook it says to run "make stage" and not "make stage-qa". So now I know that is possible, I can just run manually and don't need Poudriere at all (thank god). OUTPUT BELOW:

ums4 universal-media-server/ root~# make stage-qa
====> Running Q/A tests (stage-qa)
ums4 universal-media-server/ root~# echo $?
0
ums4 universal-media-server/ root~# head Makefile 
# Created by: Dreamcat4 <dreamcat4@gmail.com>
# $FreeBSD: head/multimedia/universal-media-server/Makefile 361621 2014-07-12 15:22:00Z pi $

PORTNAME=	universal-media-server
DISTVERSION=	4.0.1
CATEGORIES=	multimedia
MASTER_SITES=	SF/unimediaserver/Official%20Releases/Linux
DISTFILES=	UMS-${DISTVERSION}.tgz

MAINTAINER=	dreamcat4@gmail.com


Again, I say, "works fine". But heh. Now you guys have broke the rc.d script you expect me to fix it? Reality check.

I worked very hard creating this port. Most of the minor and utterly inconsequential changes you guys have made on top of that were fine. No harm done. But not this last one. It had problems and you should realise that the patch I offered now at least functionally fixes things for now. So they can be re-done right the next time. Whatever.

If you want to fix 'your way' because you think my ways wasn't good enough to begin with then by all means please go ahead. But further than that you will just loose my future contributions to FreeBSD project. I just don't have time for such pettiness / arguing for no benefit to end users. It's not going to improve their lives, in terms of the functionality of the software itself.

You guys have a right to protect your platform, but I just don't see that happening here.
Comment 7 John Marino freebsd_committer freebsd_triage 2014-08-03 14:57:20 UTC
(In reply to dreamcat4 from comment #6)
> I'm not sure I agree. The original entry is auto-generated. In other
> directories (which are not docs stuff), they don't remove without question.

When you said "auto-generated" do you mean "make makeplist"?  Because it just assigns all of the @dirrmtry and expects you to fix them (per the comment on the first line).


> If the user has modified of put something of their own files there. 

This is not allowed nor expected.  pkg is not expecting any file that it installed to be modified nor extra files to be put in its dedicated directories.  It will spew errors upon deinstall if this happens.  /usr/local is supposed to work as if it's mounted ready only

> So how
> is docs directory different? Also: that only matters IF the contents of the
> directory has been added to somehow. But the files put in there are static
> and we would not expect the directory to be non-empty. I just don't get it.

What don't you get?
There is a packing manifest.  It removes all the files in the directory and then unconditionally removes the directory.  Unless the directory is shared with one or more ports (e.g. fonts, desktop icons, etc) then you unconditionally remove it and you want to see an error occur if it's not empty (because it's expected to be empty upon deinstall)

@dirrmtry is an exception, @dirrm is normal



> > DISTFILES=	UMS-${DISTVERSION}.tgz
> > 
> > That's equivalent.  You could have also defined USES+= tar:tgz though and
> > gotten rid of the extract suffix
> 
> Yes it is. It also adds an extra line / variable to the port Makefile for no
> apparent reason whatsoever.


I don't know what you mean by this.  Your change was okay, it's a bit cleaner, it just didn't fix anything.



> > I haven't see the rc script, maybe all it needs is
> > "var/run/universal-media-server" hardcoded to it.  That actually sounds like
> > an oversight that needs fixing.  Most of the other stuff I don't see as
> > mistakes.
> 
> Yeah maybe. But there were no guideline for that in Porter's Handbook (that
> I could see)> And as the variable that is set in Makefile is currently set
> to be "/var/run/..." then functionally there is was no error at the time
> this change was introduced.

I suspect there was an error that needed fixing, and the commit just simplified this at the same time.  Personally I also prefer the hardcoding to an unchanging variable.


> In the Porters handbook it says to run "make stage" and not "make stage-qa".
> So now I know that is possible, I can just run manually and don't need
> Poudriere at all (thank god). OUTPUT BELOW:

well, no, it's not a replacement for poudriere.  Particually it will not catch file system violations or missing dependencies.  But it will check plist issues.



> ums4 universal-media-server/ root~# make stage-qa
> ====> Running Q/A tests (stage-qa)
> ums4 universal-media-server/ root~# echo $?
> 0

hmm, what?
It didn't do anything. 
try make clean ; make stage ; make stage-qa


> Again, I say, "works fine". But heh. Now you guys have broke the rc.d script
> you expect me to fix it? Reality check.

No, you would be expected to make a comment on the PR that broke script and say, "hey the script is broken".  Then the committer will likely fix it.



> I worked very hard creating this port. Most of the minor and utterly
> inconsequential changes you guys have made on top of that were fine. No harm
> done. But not this last one. It had problems and you should realise that the
> patch I offered now at least functionally fixes things for now. So they can
> be re-done right the next time. Whatever.


I think you are missing the point.  You are attempting this:
1) Update the port version [ok]
2) Fix the script problem without saying that's what you're doing, and the way you did it is probably wrong because it reversed what the previous committer did.  I guarantee you he had a reason for making the change, so putting it back to your original proposal raises flags immediately, especially given the previous committer runs his changes on 8 poudriere jails (i386, amd64 x FreeBSD 8, 9, 10, head)

What this should have done (IMO) is only update the port and fix the script issue.  


> If you want to fix 'your way' because you think my ways wasn't good enough
> to begin with then by all means please go ahead. 

I'm not going to fix it at all, I'm the triage guy.

> But further than that you
> will just loose my future contributions to FreeBSD project. I just don't
> have time for such pettiness / arguing for no benefit to end users. It's not
> going to improve their lives, in terms of the functionality of the software
> itself.

So you are going to state that an contributer with very little experience and one that is unable to use poudriere on a single platform to check his submission makes no mistakes and can declare what is useless and petty to experienced committers than have to run these works through several builders.  It takes use years to learn all the things but you've got it down in 2 weeks, that's awesome.


I've asked the previous committer to review your PR.  If he does, he can tell you why the plist was changed.
Comment 8 dreamcat4 2014-08-03 15:12:49 UTC
(In reply to John Marino from comment #7)
> > ums4 universal-media-server/ root~# make stage-qa
> > ====> Running Q/A tests (stage-qa)
> > ums4 universal-media-server/ root~# echo $?
> > 0
> 
> hmm, what?
> It didn't do anything. 
> try make clean ; make stage ; make stage-qa

It's excatly the same:

ums4 universal-media-server/ root~# make clean
===>  Cleaning for universal-media-server-4.0.1
ums4 universal-media-server/ root~# make stage
===>  License GPLv2 accepted by the user
===>  Found saved configuration for universal-media-server-4.0.1
===>   universal-media-server-4.0.1 depends on file: /usr/local/sbin/pkg - found
===> Fetching all distfiles required by universal-media-server-4.0.1 for building
===>  Extracting for universal-media-server-4.0.1
=> SHA256 Checksum OK for UMS-4.0.1.tgz.
===>  Patching for universal-media-server-4.0.1
===>   universal-media-server-4.0.1 depends on shared library: libmediainfo.so - found (/usr/local/lib/libmediainfo.so.0.0.0)
===>  Configuring for universal-media-server-4.0.1
===>  Staging for universal-media-server-4.0.1
===>   universal-media-server-4.0.1 depends on file: /usr/local/bin/dcraw - found
===>   universal-media-server-4.0.1 depends on file: /usr/local/bin/ffmpeg - found
===>   universal-media-server-4.0.1 depends on file: /usr/local/bin/flac - found
===>   universal-media-server-4.0.1 depends on file: /usr/local/bin/mencoder - found
===>   universal-media-server-4.0.1 depends on file: /usr/local/bin/mplayer - found
===>   universal-media-server-4.0.1 depends on file: /usr/local/bin/tsMuxeR - found
===>   universal-media-server-4.0.1 depends on file: /usr/local/bin/vlc - found
===>   universal-media-server-4.0.1 depends on file: /usr/local/openjdk7/bin/java - found
===>   Generating temporary packing list
===> Creating users and/or groups.
====> Compressing man pages (compress-man)
===> Staging rc.d startup script(s)
ums4 universal-media-server/ root~# make stage-qa
====> Running Q/A tests (stage-qa)
ums4 universal-media-server/ root~# echo $?
0
Comment 9 John Marino freebsd_committer freebsd_triage 2014-08-03 15:20:06 UTC
I thought stage-qa ran check-plist

It seems that it doesn't.

So what is the output of "make check-plist"?
Comment 10 John Marino freebsd_committer freebsd_triage 2014-08-03 15:21:17 UTC
FTR, I expect that to pass.  It's the /var stuff I'm concerned about and check-plist probably doesn't check that.
Comment 11 John Marino freebsd_committer freebsd_triage 2014-08-03 15:23:18 UTC
Come to think of it, poudriere might be checking for leftover /var, I was under the impression stage-qa was doing that.
Comment 12 dreamcat4 2014-08-03 15:40:54 UTC
ums4 universal-media-server/ root~# make check-plist
====> Checking for pkg-plist issues (check-plist)
===> Parsing plist
===> Checking for items in STAGEDIR missing from pkg-plist
===> Checking for directories owned by MTREEs
===> Checking for directories handled by dependencies
===> Checking for items in pkg-plist which are not in STAGEDIR
===> No pkg-plist issues found (check-plist)
Comment 13 John Marino freebsd_committer freebsd_triage 2014-08-03 15:43:52 UTC
I got tired of this round-and-round so I pulled the patch and I'm building it FreeBSD 10/amd64 poudriere.  It's building java now but I didn't want to have to download this 30M tarball to do this.

The makefile patch didn't apply.  I had to change the $FreeBSD$ tag to the current value to make it apply.  Maybe svn patch will compensate, dunno, I just used "patch" to apply it.
Comment 14 John Marino freebsd_committer freebsd_triage 2014-08-03 15:45:46 UTC
All this is going to do is indicate if the proposed patch is stage safe on FreeBSD 10 (the var workaround could have been for FreeBSD 8).

It still doesn't answer the question: Why mess with the pkg-plist instead of just fixing the script or reporting the script broken on the previous bug report?
Comment 15 John Marino freebsd_committer freebsd_triage 2014-08-03 15:49:23 UTC
good news, the patch is stage safe on FreeBSD 10/amd64 - passes all checks.
Comment 16 John Marino freebsd_committer freebsd_triage 2014-08-03 15:54:13 UTC
I have a FreeBSD 8.4 jail on my FreeBSD 10 machine, so I'm going to do another run.
Comment 17 dreamcat4 2014-08-03 17:04:19 UTC
(In reply to John Marino from comment #14)
> It still doesn't answer the question: Why mess with the pkg-plist instead of
> just fixing the script or reporting the script broken on the previous bug
> report?

Yeah, agree here this question is not answered by me. But I think you can't be satisfied with my explanation, which is this: To be utterly safe in terms of FUNCTIONAL behaviour. To stay on the side of safety. Because the known tested FUNCTIONAL configuration (by me) is the old ways. (you say isn't correct).

I still don't fully understand (myself) commiter #1's ALL of his 'pkg-plist' changes in their entirety. And this is because he didn't bother to explain it (to me). He just said "take a look". And immediately closed / committed.

My Questions,
about those things I want reverted in plist (to be safer):

* For a new user install: How does removal of "mkdir -p /var/entries" lead to NO LOSS of "/var/{run,log}/universa.../" directories, needed because the 'ums' user isn't root, and does not have permission to write directly into '/var/log/' and '/var/run' ? Did he test that 'ums' user function correctly / behave properly after that ? (I honestly don't know the answer to that question).

* For committer#1 adding of new PLIST_SUB entry to cd / change @cwd and removing previous PLIST_SUBS for /var folders does not make more efficient when those removed PLIST_SUB entry were still being used in rc.d script. Whether that rc.d usage was correct or not. So why make half-finished changes and expect someone else (me) to clean up your mess, and not feel jerked around by that?

* There are no other functional breakages that I am currently aware of... Put back "to last known previous safe configuration". Which is what the attached patch will achieve. And expends minimum time / effort for all involved to do so. That's why I said in first place "just take it". Because in the scheme of things of the world.

* Allows people involved to move on to other higher-priority issues because many other things in Ports tree are more seriously broken than this.

I'm not trying to suggest that the previous changes were all wrong / unjust. But IF

* I'm not knowledgable enough to fix them. And 
* the guy who committed them made a boo-boo and didn't really do it properly. 

Then I can't seriously be expected to say "just continue to do it HIS way and I assume it'll all be fine,"

"well probably, since that was some new configuration i had never seen before and had never functionally tested myself" 

OR i could instead say:

"Here, take this last known - good patch, which I tested last week and all was working 100% fine with the log file, pid file, rc.d script, etc, etc…"

So that's what I say / said. Please don't give me extra grief about it when I wasn't the guy who fluffed it up.
Comment 18 John Marino freebsd_committer freebsd_triage 2014-08-03 17:33:59 UTC
This patch builds cleanly on 8.4 jail as well (bulk -t)
Comment 19 John Marino freebsd_committer freebsd_triage 2014-08-03 17:50:51 UTC
(In reply to dreamcat4 from comment #17)
> (In reply to John Marino from comment #14)
> > It still doesn't answer the question: Why mess with the pkg-plist instead of
> > just fixing the script or reporting the script broken on the previous bug
> > report?
> 
> Yeah, agree here this question is not answered by me. But I think you can't
> be satisfied with my explanation, which is this: To be utterly safe in terms
> of FUNCTIONAL behaviour. To stay on the side of safety. Because the known
> tested FUNCTIONAL configuration (by me) is the old ways. (you say isn't
> correct).


So in other words, there was no identified problem.  As far as anybody knows the committed pkg-plist worked fine.

 
> I still don't fully understand (myself) commiter #1's ALL of his 'pkg-plist'
> changes in their entirety. And this is because he didn't bother to explain
> it (to me). He just said "take a look". And immediately closed / committed.


He should explain his logic.


> * Allows people involved to move on to other higher-priority issues because
> many other things in Ports tree are more seriously broken than this.


Only the script was broken, so only the script needs to be fixed.


> * the guy who committed them made a boo-boo and didn't really do it
> properly. 

He didn't see that you were using PLIST_SUBS in the RC script.  Simple oversight.


> Then I can't seriously be expected to say "just continue to do it HIS way
> and I assume it'll all be fine,"
> 
> "well probably, since that was some new configuration i had never seen
> before and had never functionally tested myself" 
> 
> OR i could instead say:
> 
> "Here, take this last known - good patch, which I tested last week and all
> was working 100% fine with the log file, pid file, rc.d script, etc, etc…"
> 
> So that's what I say / said. Please don't give me extra grief about it when
> I wasn't the guy who fluffed it up.


Part of the issue is you are now mixing bugs.  It would have been better to reopen the PR (yes, you can do that), announce that the script was busted and ask what the reason for the pkg-plist change was.

Once that was resolved, then you should have opened this PR to a simple version upgrade.

In fact, you can still do that.


I am going to assign this PR to the guy that did that last commit, with the recommendation that he process this PR.  Assuming that he has good rationale for the pkg-plist change and it works, then I would recommend that this PR only fix the script and do the update.
Comment 20 dreamcat4 2014-08-03 18:41:26 UTC
(In reply to John Marino from comment #19)
> So in other words, there was no identified problem.

No, incorrect. The PLIST_SUB is missing and still referenced in the rc.d script. That is in terms of fact "an identified problem". If you mean "well it's not proven to be a functional breakage". Which to that I ask: what the heck functional testing did this guy do before publicly committing his changes to be sure that there wasn't a breakage?

Less thorough testing than I had previously done, that's for sure.

> > * Allows people involved to move on to other higher-priority issues because
> > many other things in Ports tree are more seriously broken than this.
> 
> 
> Only the script was broken, so only the script needs to be fixed.

"Only the script is broken". So you are effectively saying (by omission) that the removal of the mkdir -p entries does not need to be fixed. Or it should be fixed only within the rc.d and not elsewhere. Because I'm not sure what you think is gonna happen when those /var folders are missing as things currently stand…
Comment 21 John Marino freebsd_committer freebsd_triage 2014-08-03 18:58:09 UTC
(In reply to dreamcat4 from comment #20)
> (In reply to John Marino from comment #19)
> > So in other words, there was no identified problem.

You can't drop the second sentence to change the context.  I qualified it as the "no problem with the pkg-plist".  I mentioned the script issue multiple times. 

> 
> No, incorrect. The PLIST_SUB is missing and still referenced in the rc.d
> script. That is in terms of fact "an identified problem". If you mean "well
> it's not proven to be a functional breakage". Which to that I ask: what the
> heck functional testing did this guy do before publicly committing his
> changes to be sure that there wasn't a breakage?

OTHER THAN THE SCRIPT, IS SOMETHING ELSE BROKEN?


> "Only the script is broken". So you are effectively saying (by omission)
> that the removal of the mkdir -p entries does not need to be fixed. 

You tell me.  Did something malfunction due to their removal?

> Or it
> should be fixed only within the rc.d and not elsewhere. Because I'm not sure
> what you think is gonna happen when those /var folders are missing as things
> currently stand…

I have been working under the assumption everything will work when the script is fixed.  The script is going to point to the right /var folders.


I want out of this conversation.  I passed the PR to robak for him to determine what problems exist and what the best approach is.  He obviously had a reason to change the PLIST so I'm not going to approve reverting it.  He can though.
Comment 22 John Marino freebsd_committer freebsd_triage 2014-08-04 10:50:08 UTC
Ok, robak and I have been looking at this.

1) The pkg-plist change he made was wrong (it missing the mkdir creation)
2) The original pkg-plist commands are also wrong, you can't @dirrmtry /var/db/$PORTNAME  (that tries /usr/local/var/db/$PORTNAME

3) The rc script is completely wrong, it's not an rc script at all.  there are directory creations, copying, all sorts of wrong stuff
4) the sample config files should be at /usr/local/etc/ ... not /var/db/... , that's completely unconventional.
5) which means @sample needs to be used.

It's been busted the entire time, ever since the first commit.
Comment 23 John Marino freebsd_committer freebsd_triage 2014-08-04 11:06:42 UTC
The rc script has to be completely reworked, we're looking at helping you with this.

We need to know the intended purpose of /var/db/$PORTNAME - what that only to host configuration files?  Or will the s/w use it for some sort of database?
Comment 24 Bartek Rutkowski freebsd_committer freebsd_triage 2014-08-05 12:59:28 UTC
Created attachment 145394 [details]
universal-media-server.diff

universal-media-server.diff
Comment 25 Bartek Rutkowski freebsd_committer freebsd_triage 2014-08-05 13:02:08 UTC
I've added a patch to current port version that should fix the issues you've reported (for which I am sorry, they werent intentional of course) and make the port even more clear and easy to work with.

As marino said, you still need to fix the rc script, so please, test this patch and let us know if the issues are gone, and post the patch with fixed rc script, so we could have a proper, elegant and working port, and after that we can think about updating it to newer version.
Comment 26 John Marino freebsd_committer freebsd_triage 2014-08-05 13:10:23 UTC
(In reply to Bartek Rutkowski from comment #25)
> I've added a patch to current port version that should fix the issues you've
> reported (for which I am sorry, they weren't intentional of course) and make
> the port even more clear and easy to work with.

Actually, the reported problem of the script wasn't addressed: The PLIST_SUB are still removed.

The text %%UMS_PROFILE_PATH%% , %%UMS_LOG_DIR%%, %%UMS_PID_DIR%% all need to be replaced by /var/<something>/universal-media-server

The universal_media_server_prestart() is a mess.
1) you don't create directories with an RC script
2) you copied from "conf" to conf.sample" instead of "conf.sample" to "conf"
3) that's handled by the pkg-plist @sample command

note, we moved the location of the conf files so @sample would work, and put it in it's conventionally location.


This RC script is doing a lot of unconventional non-FreeBSD stuff.  It needs to be reworked but robak didn't know about what you were technically trying to accomplish.

Can you make those string replacements above and fix the prestart, and then robak can advice you on some of the other problems (e.g. use of pkill and manually manipulating pid file etc)
Comment 27 John Marino freebsd_committer freebsd_triage 2014-08-05 13:15:17 UTC
(In reply to John Marino from comment #23)
> The rc script has to be completely reworked, we're looking at helping you
> with this.
> 
> We need to know the intended purpose of /var/db/$PORTNAME - what that only
> to host configuration files?  Or will the s/w use it for some sort of
> database?

We left /var/db/* in place with the assumption it's used for more than configure files (which are no longer installed there) so tell us if this is true or not.
Comment 28 John Marino freebsd_committer freebsd_triage 2014-08-05 13:24:53 UTC
(In reply to John Marino from comment #26)
> The text %%UMS_PROFILE_PATH%% , %%UMS_LOG_DIR%%, %%UMS_PID_DIR%% all need to
> be replaced by /var/<something>/universal-media-server

I guess an alternative solution would be to add these back to PLIST_SUB but they aren't used by PLIST and the values are fixed.  The .in script would be more readable without the %% variables but either solution would achieve the same result.
Comment 29 John Marino freebsd_committer freebsd_triage 2014-08-07 09:18:56 UTC
dreamcat,

As you can see, the new patch only fixes the current version minus the RC script.  The port is currently broken so we need to apply the patch.  I guess if we don't hear back soon we'll at least try to fix the prestart and template issues in the rc_script to improve the situation.

You told us not to "muck this up" but the rc script came mucked up (Robak only made it worse) so being silent now after telling how trivial we are isn't a good look.  We're trying to fix this port despite all that.
Comment 30 John Marino freebsd_committer freebsd_triage 2014-08-19 15:31:33 UTC
This weekend I started looking at this to commit the patch, but I got stuck on the prestart target of the RC script.

I really have to know how to shift the UMS from looking for conf files at /var/db and instead at /usr/local/etc/ (the normal spot)

I also need to know if anything else goes in /var/db.


We are trying to help, ignoring this in a huff isn't helping.
Comment 31 John Marino freebsd_committer freebsd_triage 2014-09-07 11:01:11 UTC
within an hour, I'm going to commit the diff (so it's less broken) and at the same time mark this port BROKEN.  We can't progress when the maintainer is being non-responsive.
Comment 32 commit-hook freebsd_committer freebsd_triage 2014-09-07 11:37:55 UTC
A commit references this bug:

Author: marino
Date: Sun Sep  7 11:37:44 UTC 2014
New revision: 367523
URL: http://svnweb.freebsd.org/changeset/ports/367523

Log:
  multimedia/universal-media-server: partially fix, but mark BROKEN

  This port has had issues every since it was introduced, but the previous
  commit actually made them worse.  This commit fixes the problems that were
  introduced then, but the RC script is currently non-functional.

  The maintainer has not been responding to repeated requests for help to
  fix the RC script, so I'm going to commit the fixes we have and mark the
  port broken now (as I was tempted to do over a month ago).

  PR:	192347

Changes:
  head/multimedia/universal-media-server/Makefile
  head/multimedia/universal-media-server/pkg-plist
Comment 33 John Marino freebsd_committer freebsd_triage 2014-09-07 11:50:08 UTC
robak,
I recommend that you wait 4 weeks on this PR.  If dreamcat4 doesn't engage by then with the objective to get the RC script functional (everything, not just prestart target), deprecate and mark this port for removal for 4 weeks after that.
Comment 34 dreamcat4 2014-09-07 16:40:14 UTC
Hi guys. I'm sorry not to have respond on this. However last time I engaged in this discussion having to explain all those little details stressed me out so much. And ended up with an eye infection only a few hours later. So once that happened to me it didn't seem very smart to continue being in this situation.

At the same time, I also felt positive that you guys are smart enough / can manage to sort out such remaining issue by yourselves without my further help or interference. Whatever my objections or concerns were at the time, they were dismissed by others and after all my own opinion aught not to be the last word.

And sorry for not checking up afterwards. But with this new bug tracker system I also kindda assumed @marino or @robak either of you would be in a position to assign something back directly on me by changing the "Assigned To" field, should my evolvement be needed again.

I'm entirely happy for others here to override my say-so and mark my patch as "rejected, not good enough / whatever" and close this PR if you know better / have a better opinion in regards to what is best for this port.

I'm also very sorry to say - I still haven't read any further comments today either. I guess I am still not entirely convinced it's worth it to come back here especially in regards to the stress involved. Not that I ever imagined (starting out) that it would end up being so stressful to begin with. I mean, apart from the responsibilities / obligations, this stuff still aught to be fun too right? And not just for myself, but for everybody who is involved.
Comment 35 John Marino freebsd_committer freebsd_triage 2014-09-07 16:46:40 UTC
Here's the synopsis:

If I had the knowledge to fix the RC script, I would have done it weeks ago.  I've asked several specific questions about it that require your answers.  Until then, we are stuck.   I was very reluctant to mark this BROKEN, but it is and it needs your attention.

After you recuperate physically and mentally, please re-read everything you missed and try to help us help you.  Many of the RC script targets are doing strange things (in BSD-land) and the prestart target is flat out broken, partially due to the relocation of the conf files to their proper location.

So we are stuck without you.
Comment 36 commit-hook freebsd_committer freebsd_triage 2014-10-16 14:13:47 UTC
A commit references this bug:

Author: robak
Date: Thu Oct 16 14:13:37 UTC 2014
New revision: 371008
URL: https://svnweb.freebsd.org/changeset/ports/371008

Log:
  multimedia/universal-media-server: set EXPIRATION_DATE to 2015-01-16

  - The port is badly broken and its maintainer has no interest in fixing
  it despite offered help. It cant be fixed without deep knowledge of the
  software and therefore it cant be fixed without maintainer's help

  PR:		192347
  Approved by:	marino (mentor)

Changes:
  head/multimedia/universal-media-server/Makefile
Comment 37 John Marino freebsd_committer freebsd_triage 2014-10-16 15:07:50 UTC
I probably would have worded this softer, something like "it appears the maintainer may have lost interest..."

For fun, I did re-read the last entry from the maintainer: " I am still not entirely convinced it's worth it to come back here especially in regards to the stress involved. Not that I ever imagined (starting out) that it would end up being so stressful to begin with."

So he spends a couple of weeks learning how to port, he makes understandable mistakes, but when informed about them, he calls everyone in the project idiots and implies that he knows better with two weeks experience then others with 4+ years of experience.  I can see how that approach can end up being stressful, especially coupled with the belief that nobody can or should question the maintainer work.

Anyway, he had over a month to cool down, re-read everything he didn't bother to read before and tackle the last bit since the rest was fixed for him.  I guess that's not going to happen.

I'll wait one more week, then I'll de-CC myself off this ticket so I don't need to look at it anymore.
Comment 38 John Marino freebsd_committer freebsd_triage 2015-02-18 15:35:47 UTC
Port removed 17 Jan 2015