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

handle sym links in when calculating repo size #4305

Merged
merged 3 commits into from
Oct 25, 2017
Merged

handle sym links in when calculating repo size #4305

merged 3 commits into from
Oct 25, 2017

Conversation

leerspace
Copy link
Contributor

repo/fsrepo: follow symbolic links when calculating RepoSize

Now if the IPFS path is a symbolic it will be followed before
calculating the repo size.

License: MIT
Signed-off-by: John Reed john@re2d.xyz

repo/fsrepo: follow symbolic links when calculating RepoSize

Now if the IPFS path is a symbolic it will be followed before
calculating the repo size.

License: MIT
Signed-off-by: John Reed <john@re2d.xyz>
@leerspace
Copy link
Contributor Author

Fixes #4056

@leerspace
Copy link
Contributor Author

I'm confused by the GitCop failures since the commit already has the license and signed-off-by trailers.

@whyrusleeping
Copy link
Member

@leerspace sometimes it gets upset because it doesnt perfectly match its regex. If you have the right content, then just ignore it.

@Kubuxu
Copy link
Member

Kubuxu commented Oct 21, 2017

If it isn't too much to ask for, a sharness test for it would be great.

@leerspace
Copy link
Contributor Author

leerspace commented Oct 21, 2017

I started working on the sharness test, but now I'm trying to figure out why every time I run ipfs repo stat the RepoSize gets larger by about ~1kB. This is even with no daemon running.

edit: this might be getting outside the scope of this PR a bit, but based on the output of find -type f | xargs md5sum in IPFS_PATH every time I run ipfs repo stat with badger it's updating a badgerds/*.vlog file, creating a new badgerds/*.sst file, and updating the badgerds/MANIFEST file.

With leveldb/flatfs, it creates a new datastore/*.ldb file and updates a datastore/*.log file.

I'm not sure if this is desired behavior (I'm guessing not), but either way I'll have to think of a good way to write the tests when the output of ipfs repo stat is unstable.

@Kubuxu
Copy link
Member

Kubuxu commented Oct 21, 2017

@leerspace this is the behaviour of those DBs, every time they are opened they create new logfile and if there are X of them (or other condition) they compact. I would suggest adding few hundreds of KiB to ipfs in sharness and checking if size is larger than X.

@Kubuxu
Copy link
Member

Kubuxu commented Oct 21, 2017

Also according to go fmt this code is not formatted correctly.

@whyrusleeping
Copy link
Member

@leerspace starting from a fresh repo (as in a test) should yield the same size every run (we have tests that test this). I would just pick a number, run the test, see what it actually was in the test failure output and then adjust the value in your test assertion.

@whyrusleeping
Copy link
Member

Though I definitely agree that running ipfs repo stat with no daemon running should not affect the size of the repo.

gofmt fixes to fsrepo.go

The sharness test checks that IPFS_PATHS that are symbolically linked to another directory are returning an appropriate size

License: MIT
Signed-off-by: John Reed <john@re2d.xyz>
@leerspace
Copy link
Contributor Author

I just committed the gofmt fix and a sharness test for this.

@whyrusleeping
Copy link
Member

@leerspace looks like you might not have set the executable bit on that test file. the test runners arent very happy trying to run it

setting the executable bit on sharness test

License: MIT
Signed-off-by: John Reed <john@re2d.xyz>
@leerspace
Copy link
Contributor Author

@whyrusleeping sorry about that. I think it should be fixed now. I did this on Windows, but fortunately there's a way to set this on Windows.

@whyrusleeping whyrusleeping merged commit 5923540 into ipfs:master Oct 25, 2017
@leerspace leerspace deleted the fix/repo-stat-sym-link branch November 3, 2017 23:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants