Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#55241 closed defect (duplicate)

base: MediaInfo: fails to upgrade in normal `port upgrade outdated` run

Reported by: Ionic (Mihai Moldovan) Owned by:
Priority: Normal Milestone:
Component: base Version: 2.4.99
Keywords: Cc: ctreleaven (Craig Treleaven)
Port: MediaInfo

Description

Port names are case sensitive.

Since the mediainfo port has been renamed to MediaInfo (and updated in the same context, which is fine), port upgrade outdated will not recognize this upgrade and carry it out.

Weirdly, port outdated is not helpful in this case:

The following installed ports are outdated:
[...]
mediainfo                      0.7.99_0 < 17.10_1

port upgrade outdated:

Nothing to upgrade.

Upgrading the port "manually" via port upgrade mediainfo works, though.

We probably should decide whether we want port names to be case sensitive or not and what to do in such edge cases.

Change History (16)

comment:1 Changed 6 years ago by ctreleaven (Craig Treleaven)

I suppose I could add a stub 'replaced_by MediaInfo' port to work around this. Are there any negative consequences to that?

comment:2 Changed 6 years ago by Ionic (Mihai Moldovan)

No, you can't! Please don't do that!

This would make the ports tree uncheckoutable on case insensitive file systems, which (arguably) most MacPorts users are using.

We have to fix this in base.

comment:3 Changed 6 years ago by ctreleaven (Craig Treleaven)

Even adding it as a subport in the existing portfile?

comment:4 Changed 6 years ago by Ionic (Mihai Moldovan)

Hm, or well, if you write it as a subport into the same Portfile, that *might* work, but there's also a high chance to completely break everything, since it looks like sometimes we handle port names case-sensitively and sometimes we don't. Bad idea, I'd say.

Oh, and likewise renaming it back to mediainfo is a bad idea as well, since in that case users who didn't have mediainfo installed and then installed MediaInfo (most likely as a dependency) or "manually" upgraded to the MediaInfo port won't be able to "upgrade" to the mediainfo port.

Last edited 6 years ago by Ionic (Mihai Moldovan) (previous) (diff)

comment:5 Changed 6 years ago by Ionic (Mihai Moldovan)

Generally it seems that we're treating data that users submitted via the command line case-insensitively (which would explain why port upgrade mediainfo finds the MediaInfo port and upgrades it), while all operations (including dependency resolving - see my previous commits that fixed MediaInfoLib trace mode builds) not involving user input handle port names case sensitively. This is now breaking our necks.

comment:6 Changed 6 years ago by ctreleaven (Craig Treleaven)

BTW, port lint doesn't like the naming difference:

$ port lint --nitpick
--->  Verifying Portfile for MediaInfo
Error: Portfile directory mediainfo does not match port name MediaInfo
--->  1 errors and 0 warnings found.

Incidentally, I suppose I could just override the name field that is coming from github.setup back to "mediainfo". Will that throw the port index into a tail spin?

comment:7 Changed 6 years ago by Ionic (Mihai Moldovan)

No, not a tail spin as such, but that would cause yet another issue, as I explained in comment:4

Last edited 6 years ago by ryandesign (Ryan Carsten Schmidt) (previous) (diff)

comment:8 in reply to:  description ; Changed 6 years ago by ryandesign (Ryan Carsten Schmidt)

Replying to Ionic:

The following installed ports are outdated:
[...]
mediainfo                      0.7.99_0 < 17.10_1

port upgrade outdated:

Nothing to upgrade.

Upgrading the port "manually" via port upgrade mediainfo works, though.

We probably should decide whether we want port names to be case sensitive or not and what to do in such edge cases.

We want port names to be handled case-insensitively. There have been numerous case-only renames of ports in the past and we want to continue to allow that to happen in the future if needed. port upgrade outdated should be fixed to handle this.

Replying to ctreleaven:

BTW, port lint doesn't like the naming difference:

$ port lint --nitpick
--->  Verifying Portfile for MediaInfo
Error: Portfile directory mediainfo does not match port name MediaInfo
--->  1 errors and 0 warnings found.

Incidentally, I suppose I could just override the name field that is coming from github.setup back to "mediainfo". Will that throw the port index into a tail spin?

If you meant to change the case of the port name, then fix the case of the directory to match. If you did not mean to change the case of the port name, put the port name back to the old case.

comment:9 in reply to:  8 Changed 6 years ago by Ionic (Mihai Moldovan)

Replying to ryandesign:

We want port names to be handled case-insensitively. There have been numerous case-only renames of ports in the past and we want to continue to allow that to happen in the future if needed. port upgrade outdated should be fixed to handle this.

I don't know which part is failing here. It seems that medialib doesn't have a directory matching its name and then there was the case-only rename.

Activating the old version of mediainfo (incidentally lowercase), renaming the port's directory to MediaInfo and regenerating the PortIndex leads to the same symptom - port outdated lists the port, but port upgrade outdated doesn't see any upgradable ports.

Base definitely isn't handling this well.

Also, I've noticed that dependencies sometimes are case sensitive. port:ZenLib != port:zenlib - at least for trace mode. At least base tried building ZenLib before MediaInfoLib when the dependency was only written as port:zenlib in MediaInfoLib, so I figure only trace mode is not handling this very well, with other components being more resilient.

Which reminds me, the dependency is also "faulty" in MediaInfo itself, so I'll fix it up as well.

If you meant to change the case of the port name, then fix the case of the directory to match. If you did not mean to change the case of the port name, put the port name back to the old case.

As already pointed out, that's a bad idea IMHO, since in that case we'll just shift the problem to new installations that pulled in the MediaInfo name (and people who already upgraded to MediaInfo "manually").

comment:10 Changed 6 years ago by jmroot (Joshua Root)

So to be clear, the outdated pseudo-portname expands to something that doesn't include mediainfo in this case? (port echo outdated)

Also is this on a case-sensitive filesystem?

comment:11 Changed 6 years ago by Ionic (Mihai Moldovan)

No, it's the other way around.

Both port outdated and port echo outdated expand to something that list the port as outdated, but port upgrade outdated doesn't upgrade/see it.

No, this is on a case insensitive file system, as is the case for most OS X users.

comment:12 Changed 6 years ago by jmroot (Joshua Root)

Resolution: duplicate
Status: newclosed

Dupe of #50518. (Actual command being run was port upgrade outdated and not GitX.)

comment:13 in reply to:  8 Changed 6 years ago by jmroot (Joshua Root)

Replying to ryandesign:

If you meant to change the case of the port name, then fix the case of the directory to match. If you did not mean to change the case of the port name, put the port name back to the old case.

Agreed, please change one or the other so they match.

comment:14 Changed 6 years ago by ctreleaven (Craig Treleaven)

comment:15 Changed 6 years ago by ctreleaven (Craig Treleaven)

In fafd589a4f9bcfe057d3e51bfee141794712e242/macports-ports:

mediainfolib: change from CamelCase to lowercase

See: #55241

comment:16 Changed 6 years ago by ctreleaven (Craig Treleaven)

Note: See TracTickets for help on using tickets.