Fix/stage symlinked files #102

evgmik · 2021-02-27T23:33:42Z

Fixes #101

Also fixes exotic case when now to be staged files are detected (i.e. we asked to stage symlinked directory), in this case there were endless wait for pbars

sahib · 2021-02-28T11:35:05Z

cmd/fs_handlers.go

+				// Since Walk does not travel symlinks, there is no point
+				// to stage links pointing to a directory. None of its files
+				// will be walked and staged.
+				log.Warningf("Not staging %v which point to a non regular file: %v %v", childPath, info.Mode(), resolvedPath)


Isn't that kinda inconsistent? We follow symlinks for files, but not directories? That doesn't really follow the principle of least surprise. Also, I would prefer to have an option to ignore symlinks altogether, which disables this behavior. Sometimes I don't want to follow symbolic links that might me take elsewhere.

Yep, I am not proud of it. The problem is that filepath.Walk by design does not follow symlinks to the directories. Fixing it would require a use of a different library.

Shall I dig in that direction?

I will work on ignore symlinks option as well.

Fixing it would require a use of a different library.

Why that? When you encounter a (valid) symbolic link to a directory, you simply call filepath.Walk on that resolved directory path. I agree that it might get a bit clumsy, but it will work. A better option might be ioutil.ReadDir() and use that in a recursive function: When encountering a directory (either regular one or resolved from a symbolic link) one would call the function that uses ioutil.ReadDir().

Care must be taken to limit the recursion depth, since symlinks can result in loops. But that applies to filepath.Walk too...

redone in series of patches. It is not as elegant as I wished. But it also simplified staging logic: no need to create dirs, internal Stage creates all necessary parent folders. Also all traveling is in one place.

Side effect: empty dirs will not be created. Do we want them? It would be relatively quick fix.

sahib · 2021-02-28T11:37:45Z

cmd/fs_handlers.go

+					log.Warningf("%v symlink points to non existing file or directory", childPath)
+					return nil
+				}
+				if err.Error() == "EvalSymlinks: too many links" {


Can't we just return the error instead of remapping the error to a log message that kinda says the same as the error? I mean for the logic it shouldn't matter if the file does not exist, is a cycle or failed for e.g. permission issues.

If we return a error, it will interrupt the rest of the staging. With a warning, we let user know that the request is impossible for only this link.

I tried both ways. It presents less surprises to a user if we batch stage as much as possible, and skip things which brig does not support vs stopping at the first filesystem incompatible link.

Would you agree?

Another option is to process errors in the caller and decide which is critical and which is not there. This way, we can return a non zero exit code, so a user can do some cli logic around to decide if it is mission critical or not.

I really need a second opinion here.

Would you agree?

Hmm, don't fully agree. Not so much about whether traversal should be stopped or not (that might be preference thing), but rather how the current error handling is implemented:

You ignore the error when symlink is broken or results in a loop.

You error out if the symlink could not be read for other reasons (permissions etc.)

I don't see a clear line between either cases, therefore I'd vote to handle them equally. In both cases it results in at least one file that could not be added. Handling equally would also rid you from having to add branches for specific error causes.

About whether to stop or not: Maybe that could be an option too (--ignore-errors) which would apply to regular files and directories too (if they could not be read due to whatever reasons). It would not be the default, so we would error out to inform users that something is wrong. My thinking for the option is that there might be different use cases:

"Live" usage where a user reads the log messages. Users might wish to ignore errors if they deem it safe.

"Automated" usage by a script that just fires the command. Errors should normally lead to a script abort here.

Another option is to process errors in the caller and decide which is critical and which is not there. This way, we can return a non zero exit code, so a user can do some cli logic around to decide if it is mission critical or not.

Would probably require interactivity (i.e. ask on the cmdline), which I'm not a big fan of in cli tools.

You logic is more consistent. Error is a error. I will modify accordingly.

It is a bit wasteful for a single file stage, but it would allow to handle staging option in one place

sahib

Seems like the fix gets more complicated 😞

Also, did we agree on an option to follow links and/or to ignore errors?

sahib · 2021-03-02T07:12:03Z

cmd/fs_handlers.go

+			}
+			childPath = resolvedPath
+			if info.Mode().IsDir() {
+				toBeDereferenced = append(toBeDereferenced, stagePair{childPath, repoPath})


More a musing: I'm a little confused by the usage of this toBeDereferenced slice. What would stop you from calling walk() again directly?

Also after some more thinking I found another issue: This method can lead to adding the same file more than once. I mentioned earlier that symbolic links transform a file tree into a graph. This leads to the possibility of encountering the same node more than once while traversing (i.e. directory symlink points one directory level up: ..).

So maybe it's actually worth looking at libraries like symwalk which claim to handle this stdlib weakness. Just this disclaimer is a bit embarassing:

CAVEAT: Note that symwalk.Walk does not terminate if there are any non-terminating loops in the file structure.

Maybe there are better libraries also :)

I looked at the symwalk code and that CAVEAT inspired me to write our own,
it is not very different, from our walk. The main change was to track depth of recursion. Honestly, speaking I would say having that walk in the library is somewhat overkill. It is a typical undergraduate problem, with 10 LOC solution.

Admittedly, I made it a bit more complicated. As you suggest, instead of collecting toBeDereferenced, I could have jump to the walk right away. It would be cleaner, and I will redo it accordingly.

Traveling same node more than once is a feature of the symlinks. So this is a necessary evil. I even tested our code dir1/dir2 --> ../dir1, it creates quite a long unwraped tree (limited by the set depth).

sahib · 2021-03-02T07:12:46Z

cmd/fs_handlers.go

+			if err != nil {
+				return fmt.Errorf("Failed to resolve: %v: %v", childPath, err)
+			}
+			info, err = os.Stat(resolvedPath)


Another evil thing: Symbolic links can resolve to more symbolic links. They're really a hostile concept to
implement.

Nevermind, I was thinking about os.Lstat() - filepath.Evalsymlinks should handle this.

sahib · 2021-03-02T07:18:13Z

I guess the symwalk package can be copied and adjusted to support:

A maximum depth of traversal.
Not visiting the same node (identified by inode/device_id) more than once.

evgmik · 2021-03-02T15:39:17Z

I guess the symwalk package can be copied and adjusted to support:
* A maximum depth of traversal.

What we already have does it. I run some test and I see no obvious bugs in it.

* Not visiting the same node (identified by inode/device_id) more than once.

I would disagree on this. I think this is the feature of symlinks: multiple links can point to the same inode. If we remove it, it would be very similar as not having symlinks at all (except the cases when they point outside of the staging root).

My use case: having a template dir (with makefiles, styles, and common configs). I have quite a lot of link to this template dir within a working tree.
Think 'lectute template' to which weekly lectures point.

evgmik · 2021-03-02T15:43:02Z

Also, did we agree on an option to follow links and/or to ignore errors?

This is in my TODO list. I was looking for a general approval of the symlink handling, before adding extra options.

An embarrassing story: I spent 5 hours figuring out how to append a slice returned by a function. The go error reporting was very unclear about it.
... did the work, but it took me long time.

sahib · 2021-03-02T17:41:45Z

I guess the symwalk package can be copied and adjusted to support:
* A maximum depth of traversal.
What we already have does it. I run some test and I see no obvious bugs in it.
* Not visiting the same node (identified by inode/device_id) more than once.
I would disagree on this. I think this is the feature of symlinks: multiple links can point to the same inode. If we remove it, it would be very similar as not having symlinks at all (except the cases when they point outside of the staging root).

My use case: having a template dir (with makefiles, styles, and common configs). I have quite a lot of link to this template dir within a working tree. Think 'lectute template' to which weekly lectures point.

Hmm, you have a point here. I was more worried about such pathological (hah, "path") cases:

λ werkbank sub → cd /tmp && mkdir sub && cd sub
λ werkbank sub → dd if=/dev/urandom of=file.big bs=1M count=100
λ werkbank sub → ln -s . link
λ werkbank sub → ls
file.big  link
λ werkbank sub → cd link
λ werkbank link → cd link
λ werkbank link → cd link
λ werkbank link → cd link
λ werkbank link → cd link
λ werkbank link → cd link
λ werkbank link → cd link
λ werkbank link → cd link
λ werkbank link → pwd
/tmp/sub/link/link/link/link/link/link/link/link

...in that case the current code would add an infinitely nested structure, each level with file.big in it. You could say that this is normally not true, but I consider such things a security issue because you could easily DDoS a machine with brig if you somehow manage to create a symbolic link and make it stage something.

So both solutions lack something:

Ignoring cycles limits valid use cases.
Not ignoring cycles opens room for malicious input.

I really don't like the concept of symbolic links. Without supporting some sort of "link" in brig we can't really cover both cases and that is really out of scope. Maybe one way would be to "burn" a directory-symlink, so we can traverse it only once and won't the next time. Since I can't come up with a good solution here you have my go to proceed. Might play around at some point, so please leave a TODO with the problem described above.

An embarrassing story: I spent 5 hours figuring out how to append a slice returned by a function. The go error reporting was very unclear about it. ... did the work, but it took me long time.

Sorry to hear that 😁 The ... thing isn't very intuitive also. I think I wrote a for loop around it the first time.

evgmik · 2021-03-03T03:42:09Z

λ werkbank sub → cd /tmp && mkdir sub && cd sub
λ werkbank sub → dd if=/dev/urandom of=file.big bs=1M count=100
λ werkbank sub → ln -s . link
λ werkbank sub → ls

file.big link

/tmp/sub/link/link/link/link/link/link/link/link


...in that case the current code would add an infinitely nested structure, each level with `file.big` in it. You could say that this is normally not true, but I consider such things a security issue because you could easily DDoS a machine with brig if you somehow manage to create a symbolic link and make it stage something.

Just small correction: we already have recursion depth protection with the current code. The limit is set to 255 in the current walk code, see
6a1e67f line 73. What would be nice is to reference file.big to the same backend hash. Unfortunately, we lost the automatic way for this when we switched to random encryption key from the content generated one.

But since I collect long list of files toBeStaged I can see which of them dereferenced to the same local file. And just copy the node info. This will save quite a lot of space at the backend for some scenarios, it also could speed up staging for heavily linked cases.

We might even attempt it with directories, but this is scary. I am not sure our linker will survive it.

So both solutions lack something:
* Ignoring cycles limits valid use cases.

* Not ignoring cycles opens room for malicious input.
I really don't like the concept of symbolic links. Without supporting some sort of "link" in brig we can't really cover both cases and that is really out of scope. Maybe one way would be to "burn" a directory-symlink, so we can traverse it only once and won't the next time. Since I can't come up with a good solution here you have my go to proceed. Might play around at some point, so please leave a TODO with the problem described above.

Sure thing I will leave the TODO.

evgmik · 2021-03-07T23:49:37Z

I am still working on proper link handling.

As a side effect, it does not go into infinite recursion loops, when a link references direct or indirect link to itself.

evgmik · 2021-03-09T04:58:00Z

still working on it. In particular, on deduplication: if several repo paths points to the same content via symlinks during staging, then we should just copy repo node to new repopaths. This is how I triggered bug #105

sahib · 2021-03-09T22:51:36Z

How are you implementing the linking? Are you using fs.Copy() or do some low-level trickery? Or something different entirely more on client-side?

evgmik · 2021-03-10T00:37:58Z

How are you implementing the linking? Are you using fs.Copy() or do some low-level trickery? Or something different entirely more on client-side?

I am going to use fs.Copy(). It seems to be sufficient except it does not copy to non existing path (at least on a first glance).

If during staging there are symlinks pointing to the same file, then relevant repo path will have similar content and backend hashes. This is achieved by staging the first occurrence of the file and then copying the content via brig `Copy(src, dst)`. This also allow faster staging by reducing duplicate work of encrypting and compressing.

evgmik · 2021-03-14T04:28:33Z

Status update. Looks like I completed reflinking of the symlinked files. Next is proper treatment of the symlinked directories, then implementation for options.

sahib

Concept looks good, but I think there are some flaws...

sahib · 2021-03-14T11:00:18Z

cmd/fs_handlers.go

 				return nil
 			}
 		}

 		if info.Mode().IsRegular() {
-			toBeStaged = append(toBeStaged, stagePair{childPath, repoPath})
+			list, ok := toBeStaged[childPath]


I'm confused. You're defining equalness based on the path. This will work only in case of absolute symbolic links. Sadly, the more common form I saw are relative symbolic links. Those will lead to different paths for the same file (did I mention that I hate the concept?):

→ touch im-the-same-file-always → ln -s . sub-directory → cd sub-directory → pwd /tmp/t/sub-directory/sub-directory → stat /tmp/t/sub-directory/sub-directory/im-the-same-file-always File: /tmp/t/sub-directory/sub-directory/im-the-same-file-always Device: 30h/48d Inode: 61479275 Links: 1 ... → cd sub-directory → stat /tmp/t/sub-directory/sub-directory/sub-directory/im-the-same-file-always File: /tmp/t/sub-directory/sub-directory/sub-directory/im-the-same-file-always Device: 30h/48d Inode: 61479275 Links: 1 ...

As you can see, the path changes since it's totally valid for the same file to have several paths (-> hardlinks). The only thing that stays constant is the combination of device/inode number. You should use this as key to uniqueness.

I'm confused. You're defining equalness based on the path. This will work only in case of absolute symbolic links.

If we track only symbolic links to files, the path equality is good enough.
filepath.EvalSymlinks resolves the link to its absolute path so we are covered.

Tracking links to directories is hard as you correctly pointed out. I am working on it in a new set of patches. I think EvalSymlinks still would resolve to the absolute parent, but I need more tests for this case. Current, algorithm reject simple infinite loops due to depth control. But I want to have directory reflinking for the non abusive loop free symlinks.

Hopefully, a user is not in self destruction/harm mode, and does not do loop symlinks unless there is a good case for it. I myself cannot come up with a useful case for looped symlinks.

If we track only symbolic links to files, the path equality is good enough.
filepath.EvalSymlinks resolves the link to its absolute path so we are covered.

True. Just tested, and it seems that filepath.EvalSymlinks is clever enough to always return /tmp/t//im-the-same-file-always. It would be interesting to see how it does that, will check. I still suggest to use dev/inode instead of path due to the added benefit of detecting hardlinks.

Hopefully, a user is not in self destruction/harm mode, and does not do loop symlinks unless there is a good case for it. I myself cannot come up with a useful case for looped symlinks.

There is probably none. It's "just" a security thing from my perspective.

sahib · 2021-03-14T11:00:48Z

cmd/fs_handlers.go

+						}
+						continue
+					}
+					ctl.Copy(firstToStage, repoPath)


I think you're missing the error handling here.

ups, that was left over of copy-pasting. Added a better error handler in 49ddee9

sahib · 2021-03-14T11:02:19Z

cmd/fs_handlers.go

+						// first occurrence is staged
+						if err := ctl.Stage(pairSet.local, repoPath); err != nil {
+							fmt.Printf("failed to stage %s: %v\n", pairSet.local, err)
+							break


Does it make sense to stop all staging when there was errors? Probably also something for the error ignoring option...

Well, the failed staging might be a minor thing: someone changed a local filesystem (deleted a file or set unreadable permissions). I think it is better to report than stop staging at this stage.

Besides, I have no idea how to send an abort message from the go routine :(

sahib · 2021-03-14T11:02:58Z

cmd/fs_handlers.go

+						continue
+					}
+					ctl.Copy(firstToStage, repoPath)
+					if err := ctl.Stage(pairSet.local, repoPath); err != nil {


Why are we staging again here? Shouldn't the copy alone suffice?

My bad, leftover of copy-pasting. Note to myself: do not push commits at midnight.
Should be fixed in 49ddee9.

When symlinked files are copied (reflinked) often they point from a non existing directory. This patch takes care of this problem.

evgmik · 2021-03-15T03:05:43Z

Still working on hardlinks and dirs

evgmik · 2021-03-16T02:59:53Z

Do we really want to implement hardlinks reflinking?

To track hardlinks we have to use OS dependent package x/sys. I would argue that this is an overkill and will be hard to maintain. The price we pay is some data duplication (I start to regret our decision on a random key choice for every file, instead of content based.)

I personally never used hardlinks in real life. But I need a weighted @sahib opinion on it.

One more thing:
I decided to skip copying symlinked directories. The current algorithm walks such directories and copies its content, instead of fs.Copy(src_dir, dest_dir). But I think this is not performance crucial. The reason: it is hard to implement properly, since I need to track which dir is already staged to do its copy, so the code grows for a questionable speed up.

Other than this, I will be working on skip symlinks and continue on the minor staging errors switches/options.

sahib · 2021-03-16T19:05:49Z

Do we really want to implement hardlinks reflinking?

It's not an easy discussion. Here's my take.

I care mostly for consistency. If we support symbolic links we should also support hardlinks. To be fair, using fs.Copy is more an optimization more or less invisible to the user, but still. Good software should support all normal operation that a user encounters or, if that seems to be out of reach for whatever reason, limit itself to a sensible subset. The latter is the way that Go choose, by ignoring everything in filepath.Walk() that's not a regular file. So, let's ask ourselves if we're consistent:

Staging works for both hardlinks, symbolic links (both regular and directory) and regular files.
The copy-optimization works only for symbolic links, but only if they point to single files.
Broken links and loops are handled. That's more a security requirement though.

The second point is inconsistent, but I would be okay to let it slip since it's a non-essential optimization and it's likely that most users don't even care or know. So I'm kinda okay with the current state of not explicitly supporting for the following reasons:

There is no proper generic walk implementation that does the right thing. Maybe some day I will write one...
I don't see it as an highly important optimization right now. I think it's way to early for such detail optimization in general.

I personally never used hardlinks in real life. But I need a weighted @sahib opinion on it.

I use them quite often. For example parts of my music library are hardlinks and I use them to create "playlists" that can be easily copied to elsewhere without fear of tools that do not handle symbolic links right. But also many tools like ostree, git and many more are based on them. Personally, I see them more often than symbolic links.

To track hardlinks we have to use OS dependent package x/sys.

Well, yes. It adds a bit of complexity, but not much. One could just have a "key" type that is either dev/inode on unix or path on windows.

Other than this, I will be working on skip symlinks and continue on the minor staging errors switches/options.

👍

This done by tracking files inodes and converting them to a unique string in the form "DeviceID/Inode". This work only on systems where sys/Lstat is supported. Currently, it is various unixes and more specifically linux for this patch.

evgmik · 2021-03-19T03:01:51Z

I was convinced that hardlinks are a nice feature (should I expect anything different from rmlint author ? :)

I have added hardlinks reflinking with 9f93d01. It is tested only on linux, and on other systems should fall back to just staging a hardlinked file independently (not sure it supported by Windows at all).

Technically, it should work on any unix like system, not just linux. But I do not see the way to make a build flag similar to

// +build linux

if I replace linux with unix the system detects both inodeString declarations. But it should be a minor thing which could be tested if we get a hold of a Mac

sahib · 2021-03-19T17:05:28Z

Nice! Implementation looks okay to me. Will have deeper look once the options are also implemented.

[...] should I expect anything different from rmlint author?

Damn, my past catches up. 😄 But yes, not scanning hardlinks twice was an issue there.

Technically, it should work on any unix like system, not just linux. But I do not see the way to make a build flag similar to [...]

If I understood you right you could have the following configuration (untested, but did something similar once):

$ ls inode_*
inode_unix.go inode_other.go
$ head -1 inode_unix.go
// +build !windows
$ head -1 inode_other.go
// +build windows

Under the assumption that pretty much everything beside windows is a unix or at least has inodes. Even if that does not apply I think it's better to keep it like that so users can complain when compiling for this platform.

The previous version distinguished between linux and other, which is less general. Since inode discovery exists on many unixes not just linux.

evgmik · 2021-03-21T15:08:05Z

If I understood you right you could have the following configuration (untested, but did something similar once):
$ ls inode_*
inode_unix.go inode_other.go
$ head -1 inode_unix.go
// +build !windows
$ head -1 inode_other.go
// +build windows
Under the assumption that pretty much everything beside windows is a unix or at least has inodes. Even if that does not apply I think it's better to keep it like that so users can complain when compiling for this platform.

Excellent suggestion, implemented with 38704e1

Now, back to the staging options...

…, -c It mostly protects against broken link, or some other issues at the local (hard drive) file system.

evgmik · 2021-03-21T16:37:49Z

I have added options:

to skip symbolic links: --no-dereference, -P
to continue on error during staging: --continue-on-error, -c

@sahib I think this patch set is ready for the final review

sahib

Looks good to me overall, just two minor comments.

sahib · 2021-03-22T16:50:27Z

cmd/fs_handlers.go

-			if err := ctl.Mkdir(repoPath, true); err != nil {
-				return e.Wrapf(err, "mkdir: %s", repoPath)
+		if opt.dereference && info.Mode()&os.ModeSymlink != 0 {
+			// TODO: `brig` does not have concept of symlink


s/TODO/NOTE/g

(Not the lack of symlinks is the problem, but the lack of a good available implementation)

fixed in 2376ea1

sahib · 2021-03-22T16:55:51Z

cmd/fs_handlers.go

+						// First occurrence is staged.
+						// Stage creates all needed parent directories.
+						if err := ctl.Stage(twinsSet.localPath, repoPath); err != nil {
+							fmt.Printf("failed to stage '%s' as '%s': %v\n", twinsSet.localPath, repoPath, err)


Common guideline is that one outputs warnings and errors to stderr rather than stdout.

stdout -> stderr with d89e161

sahib · 2021-03-22T16:57:33Z

Small personal note: I have to sort out some personal stuff and and will be slow to respond/develop in the next 1-2 weeks.

evgmik · 2021-03-22T17:22:10Z

Small personal note: I have to sort out some personal stuff and and will be slow to respond/develop in the next 1-2 weeks.

No worries. I will start working on 'copy' related bugs #105 and #106

evgmik added 2 commits February 27, 2021 18:28

stage symlinks which points to a regular files

233cb1c

preventing pbars from waiting forever if the toBeStaged array is empty

9d36ee0

evgmik requested a review from sahib February 27, 2021 23:33

sahib requested changes Feb 28, 2021

View reviewed changes

evgmik added 7 commits February 28, 2021 13:00

don't do special treatment for unresloved symlinks: error is error

7f67df4

no need to create a directory, file staging creates needed dir structure

8c90bd0

somewhat ugly following symlinked dirs

b4e1fdf

recursive links dereference with recursion depth control

6a1e67f

unified staging, it is all done via the same function

6839131

It is a bit wasteful for a single file stage, but it would allow to handle staging option in one place

go fmt

2a701a0

Merge branch 'develop' into fix/stage-symlinked-files

881fa17

sahib requested changes Mar 2, 2021

View reviewed changes

evgmik added 3 commits March 8, 2021 23:01

more informative error message

a2e4cda

a cleaner recursion without temporary toBeDereference variable

f43fc5f

As a side effect, it does not go into infinite recursion loops, when a link references direct or indirect link to itself.

added TODO about handling symlinks

8f1e91f

sahib requested changes Mar 14, 2021

View reviewed changes

evgmik added 3 commits March 14, 2021 19:27

fixed inproper error handling due to copy-pasting

49ddee9

more verbose error message

3de1838

Stage creates necessary parent directories

ab61b4b

When symlinked files are copied (reflinked) often they point from a non existing directory. This patch takes care of this problem.

evgmik added 2 commits March 18, 2021 22:53

Support of reflinking for hardlinks during staging

9f93d01

This done by tracking files inodes and converting them to a unique string in the form "DeviceID/Inode". This work only on systems where sys/Lstat is supported. Currently, it is various unixes and more specifically linux for this patch.

requiring x/sys package directly

b932f3e

evgmik added 2 commits March 21, 2021 11:02

more general treatment of builds, it now distinguish windows or unix

38704e1

The previous version distinguished between linux and other, which is less general. Since inode discovery exists on many unixes not just linux.

removed debug output

246340d

evgmik added 2 commits March 21, 2021 12:13

added option to skip symbolic links: --no-dereference, -P

06e971b

added option to continue on error during staging: --continue-on-error…

4656293

…, -c It mostly protects against broken link, or some other issues at the local (hard drive) file system.

adding conditional error skipping in one more place

67793c6

sahib approved these changes Mar 22, 2021

View reviewed changes

evgmik added 2 commits March 22, 2021 13:09

s/TODO/NOTE/

2376ea1

staging error and warning messages reported to stderr

d89e161

Merge branch 'develop' into fix/stage-symlinked-files

991a2ea

evgmik merged commit 6b7eccf into develop Mar 23, 2021

evgmik deleted the fix/stage-symlinked-files branch March 23, 2021 13:23

Fix/stage symlinked files #102

Fix/stage symlinked files #102

Conversation

evgmik commented Feb 27, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sahib left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sahib Mar 2, 2021 • edited Loading

Choose a reason for hiding this comment

sahib commented Mar 2, 2021

evgmik commented Mar 2, 2021

evgmik commented Mar 2, 2021 • edited Loading

sahib commented Mar 2, 2021

evgmik commented Mar 3, 2021

evgmik commented Mar 7, 2021

evgmik commented Mar 9, 2021

sahib commented Mar 9, 2021

evgmik commented Mar 10, 2021

evgmik commented Mar 14, 2021

sahib left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evgmik Mar 14, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evgmik Mar 14, 2021 • edited Loading

Choose a reason for hiding this comment

evgmik commented Mar 15, 2021

evgmik commented Mar 16, 2021

sahib commented Mar 16, 2021

evgmik commented Mar 19, 2021 • edited Loading

sahib commented Mar 19, 2021

evgmik commented Mar 21, 2021

evgmik commented Mar 21, 2021

sahib left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sahib commented Mar 22, 2021

evgmik commented Mar 22, 2021

sahib Mar 2, 2021 •

edited

Loading

evgmik commented Mar 2, 2021 •

edited

Loading

evgmik Mar 14, 2021 •

edited

Loading

evgmik Mar 14, 2021 •

edited

Loading

evgmik commented Mar 19, 2021 •

edited

Loading