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

failed backward compatbility #81

Closed
GoogleCodeExporter opened this issue Aug 14, 2015 · 11 comments

Comments

@GoogleCodeExporter
Copy link

commented Aug 14, 2015

AFAIK, include dir "google" is still available in 2.0.1 because of backward 
compatibility and new codes should use the new include dir "sparsehash".

However, this dir "google" is not correct as backward compatibility, because 
the old version 1.12 had include dir 'google/sparsehash', not just 'google'. 
So, yes, it is broken.

In my case, PCSX2 failed to build, because it didn't find include dir. Here is 
an open thread in PCSX2 forum (just in case you want to see other report): 
http://forums.pcsx2.net/Thread-ZZogl-ERROR-Need-GL-EXT-framebuffer-object-for-mu
ltiple-render-targets

I'm currently using Sparsehash version 2.0.1 on Arch Linux 64-bit(all system 
up-to-date)

Original issue reported on code.google.com by rafael.f...@gmail.com on 14 Feb 2012 at 1:28

@GoogleCodeExporter

This comment has been minimized.

Copy link
Author

commented Aug 14, 2015

Hi Rafael,

thanks for the report. Looks like someone hit the nail on the header at the end 
of the thread:

http://forums.pcsx2.net/Thread-ZZogl-ERROR-Need-GL-EXT-framebuffer-object-for-mu
ltiple-render-targets?pid=210010#pid210010

Which says that:

#include <google/sparse_hash_map>

maps to:

#include <sparsehash/sparse_hash_map>

and

#include <google/sparsehash/sparsehashtable.h>

maps to

#include <sparsehash/internal/sparsehashtable.h>

It would have been a little strange to have migrated to 
sparsehash/sparsehash/sparsehashtable.h :)

Not sure if the fix lies in the cmake script for PCSX2. Are you using an Arch 
package of PCSX2 or sparsehash? Can you attach a full build log with the 
encountered error as well please?

Cheers,
Donovan.



Original comment by DonovanH...@gmail.com on 14 Feb 2012 at 2:07

@GoogleCodeExporter

This comment has been minimized.

Copy link
Author

commented Aug 14, 2015

[deleted comment]
@GoogleCodeExporter

This comment has been minimized.

Copy link
Author

commented Aug 14, 2015

It was actually me who posted this message on the end of the thread. Then I 
thought it would be nice warning Sparsehash team that this backward 
compatibility folder (google/) is not working.

Please note that I'm not telling you to 
'sparsehash/sparsehash/sparsehashtable.h' (ugly!), but I'm suggesting that 
moving all 'google/' content to 'google/sparsehash/', which is the old header 
folder in version 1.12, would be a less impactant way to move to the new 
'sparsehash/' directory.

Why I'm worried about this backward compatibility? My answer is:

1) Some linux distributions do not update as quickly as Archlinux (which 
updated sparsehash to 2.0.1 already). Easiest example of what I'm saying is 
Debian. So, using my example, PCSX2 will need to be able to find Sparsehash 
header in the system in both old location (google/sparsehash) for 
not-so-fast-update (ex: Debian) and new location (sparsehash) for Archlinux. 
This is feasible, but could be avoided.

2) Your code provides 'google/' for backward compatibility (if I'm not wrong), 
but it won't work, but old directory was 'google/sparsehash/*', and not 
'google/*'. If this backward compatibility is not working, or remove or fix it 
.(IMHO)

I really think the new file/directory structure is great. Just this backward 
compatibility would be nice to be working.

IMO, you should install all 'google/' content in 'google/sparsehash/' to fix 
provide a less impactant migration, always telling people that this is 
deprecated.

See some information:
- my PCSX2 build log -- see how it failed to find sparsehash header 
(http://pastebin.com/J9Bmfmn1)
- FindSparsehash.h --- used to find it, but currently using old directory (must 
fix) 
(http://code.google.com/p/pcsx2/source/browse/trunk/cmake/FindSparseHash.cmake)
- archlinux's google-sparsehash 1.12 file content (http://pastebin.com/BXsQcpDA)
- archlinux's sparsehash 2.0.1 file content (http://pastebin.com/sWvEYJtz)

Original comment by rafael.f...@gmail.com on 14 Feb 2012 at 2:48

@GoogleCodeExporter

This comment has been minimized.

Copy link
Author

commented Aug 14, 2015

Hi Rafael,

thanks for the quick reply! Possibly looks to me like the 2.0.1 Arch file 
contents have messed up the intended include folder structure:

http://code.google.com/p/sparsehash/source/browse/#svn%2Ftrunk%2Fsrc%2Fgoogle%2F
sparsehash

compare to Arch:

-rw-r--r-- root/root      1816 2012-02-12 22:19 
usr/include/google/libc_allocator_with_realloc.h
-rw-r--r-- root/root      1805 2012-02-12 22:19 
usr/include/google/hashtable-common.h
-rw-r--r-- root/root      1804 2012-02-12 22:19 
usr/include/google/sparsehashtable.h
-rw-r--r-- root/root      1803 2012-02-12 22:19 
usr/include/google/densehashtable.h

These files should be in /usr/include/google/sparsehash/ not 
/usr/include/google/ !

Is this the Arch package that reflects that file list?

https://aur.archlinux.org/packages.php?ID=56286

Not sure how this happened, but the folder structure is correct on the tarball 
that fed the Arch package from:

http://sparsehash.googlecode.com/files/sparsehash-2.0.1.tar.gz

Bear in mind that only .tar.gz, .deb and .rpm files are covered by our release 
process currently. Might be worth firing an email of to the Arch maintainer: 
mtorromeo

Hope that helps,
Donovan.

Original comment by DonovanH...@gmail.com on 14 Feb 2012 at 3:06

@GoogleCodeExporter

This comment has been minimized.

Copy link
Author

commented Aug 14, 2015

Ah, now I can see that you already had the source code adapted to install it in 
google/sparsehash.

You're right about the package URL and who is the maintainer. I'm just not sure 
it is something that he/she must fix. If you read the PKGBUILD, which is kinda 
similar to the 'rules' file of Debian, you'll see that the maintainer just made 
a simple process of './configure', 'make' and 'make install'. This should have 
installed all packages without issue.

I attached the PKGBUILD (also available online), my config.log, my build.log 
(related to build() function) and package.log (related to package() function). 
Can you take a look at it? Hope it helps to narrow down the cause.

And thanks for you help so far!

Original comment by rafael.f...@gmail.com on 14 Feb 2012 at 3:33

Attachments:

@GoogleCodeExporter

This comment has been minimized.

Copy link
Author

commented Aug 14, 2015

observation: in the package.log you can see that it deliberately installed in 
wrong folder. Could it be related to the Makefile?

Original comment by rafael.f...@gmail.com on 14 Feb 2012 at 3:42

@GoogleCodeExporter

This comment has been minimized.

Copy link
Author

commented Aug 14, 2015

Hi again!

Thanks for the detailed logs, the error can be seen in lines 31-34 of 
sparsehash-2.0.1-1-x86_64-package.log 

Looks like the "make install" macros might be messing up somewhere. I'll have a 
good look through tomorrow and let you know!

Cheers,
Donovan.

Oops, you just posted the same observation :)

Original comment by DonovanH...@gmail.com on 14 Feb 2012 at 3:53

  • Changed state: Accepted
@GoogleCodeExporter

This comment has been minimized.

Copy link
Author

commented Aug 14, 2015

Hi,

sorry for the delay in getting back! Are you able to test a build of the 
current trunk against PCSX2?

You might have to manually clear out the /usr/local/include/google folder for 
sparsehash related files.  After that:

svn checkout http://sparsehash.googlecode.com/svn/trunk/ sparsehash
cd sparsehash
./configure && make && sudo make install

and then a PCSX2 build. Not sure if this is possible for you without an Arch 
package...

The Makefile now puts the legacy internal headers in google/sparsehash as used 
to be the case.

Let me know how you get on.

Donovan.


Original comment by DonovanH...@gmail.com on 22 Feb 2012 at 9:48

  • Changed state: Started
  • Added labels: Priority-High
  • Removed labels: Priority-Medium
@GoogleCodeExporter

This comment has been minimized.

Copy link
Author

commented Aug 14, 2015

Works like a charm! Thanks a lot, Donovan!

Original comment by rafael.f...@gmail.com on 22 Feb 2012 at 11:31

@GoogleCodeExporter

This comment has been minimized.

Copy link
Author

commented Aug 14, 2015

Great! I'll do a package tomorrow and fire an email off to the Arch maintainer.

Original comment by DonovanH...@gmail.com on 22 Feb 2012 at 11:45

@GoogleCodeExporter

This comment has been minimized.

Copy link
Author

commented Aug 14, 2015

Original comment by DonovanH...@gmail.com on 24 Feb 2012 at 1:36

  • Changed state: Fixed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.